Merge pull request #64815 from dixudx/hostname_empty

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

error out empty hostname

**What this PR does / why we need it**:
For linux, the hostname is read from file `/proc/sys/kernel/hostname` directly, which can be overwritten with whitespaces.

Should error out such invalid hostnames.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes kubernetes/kubeadm#835

**Special notes for your reviewer**:
/cc luxas timothysc 

**Release note**:

```release-note
nodes: improve handling of erroneous host names
```

cmd/kube-proxy/app/server_others.go

@@ -114,7 +114,10 @@ func newProxyServer(
 	}
 
 	// Create event recorder
-	hostname := utilnode.GetHostname(config.HostnameOverride)
+	hostname, err := utilnode.GetHostname(config.HostnameOverride)
+	if err != nil {
+		return nil, err
+	}
 	eventBroadcaster := record.NewBroadcaster()
 	recorder := eventBroadcaster.NewRecorder(scheme, v1.EventSource{Component: "kube-proxy", Host: hostname})
 

cmd/kube-proxy/app/server_windows.go

@@ -72,7 +72,10 @@ func newProxyServer(config *proxyconfigapi.KubeProxyConfiguration, cleanupAndExi
 	}
 
 	// Create event recorder
-	hostname := utilnode.GetHostname(config.HostnameOverride)
+	hostname, err := utilnode.GetHostname(config.HostnameOverride)
+	if err != nil {
+		return nil, err
+	}
 	eventBroadcaster := record.NewBroadcaster()
 	recorder := eventBroadcaster.NewRecorder(scheme, v1.EventSource{Component: "kube-proxy", Host: hostname})
 

cmd/kubeadm/app/cmd/phases/certs_test.go

@@ -189,7 +189,10 @@ func TestSubCmdCertsApiServerForwardsFlags(t *testing.T) {
 		t.Fatalf("Error loading API server certificate: %v", err)
 	}
 
-	hostname := node.GetHostname("")
+	hostname, err := node.GetHostname("")
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	for i, name := range []string{hostname, "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.mycluster.local"} {
 		if APIserverCert.DNSNames[i] != name {

cmd/kubeadm/app/phases/kubelet/flags.go

@@ -46,6 +46,10 @@ type kubeletFlagsOpts struct {
 // WriteKubeletDynamicEnvFile writes a environment file with dynamic flags to the kubelet.
 // Used at "kubeadm init" and "kubeadm join" time.
 func WriteKubeletDynamicEnvFile(nodeRegOpts *kubeadmapi.NodeRegistrationOptions, featureGates map[string]bool, registerTaintsUsingFlags bool, kubeletDir string) error {
+	hostName, err := nodeutil.GetHostname("")
+	if err != nil {
+		return err
+	}
 
 	flagOpts := kubeletFlagsOpts{
 		nodeRegOpts:              nodeRegOpts,
@@ -53,7 +57,7 @@ func WriteKubeletDynamicEnvFile(nodeRegOpts *kubeadmapi.NodeRegistrationOptions,
 		registerTaintsUsingFlags: registerTaintsUsingFlags,
 		execer:          utilsexec.New(),
 		pidOfFunc:       procfs.PidOf,
-		defaultHostname: nodeutil.GetHostname(""),
+		defaultHostname: hostName,
 	}
 	stringMap := buildKubeletArgMap(flagOpts)
 	argList := kubeadmutil.BuildArgumentListFromMap(stringMap, nodeRegOpts.KubeletExtraArgs)

cmd/kubeadm/app/phases/markmaster/markmaster_test.go

@@ -108,7 +108,10 @@ func TestMarkMaster(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		hostname := node.GetHostname("")
+		hostname, err := node.GetHostname("")
+		if err != nil {
+			t.Fatalf("MarkMaster(%s): unexpected error: %v", tc.name, err)
+		}
 		masterNode := &v1.Node{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: hostname,

cmd/kubeadm/app/util/config/masterconfig.go

@@ -94,7 +94,10 @@ func SetInitDynamicDefaults(cfg *kubeadmapi.InitConfiguration) error {
 		cfg.BootstrapTokens[i].Token = token
 	}
 
-	cfg.NodeRegistration.Name = nodeutil.GetHostname(cfg.NodeRegistration.Name)
+	cfg.NodeRegistration.Name, err = nodeutil.GetHostname(cfg.NodeRegistration.Name)
+	if err != nil {
+		return err
+	}
 
 	// Only if the slice is nil, we should append the master taint. This allows the user to specify an empty slice for no default master taint
 	if cfg.NodeRegistration.Taints == nil {

cmd/kubeadm/app/util/config/nodeconfig.go

@@ -32,7 +32,11 @@ import (
 
 // SetJoinDynamicDefaults checks and sets configuration values for the JoinConfiguration object
 func SetJoinDynamicDefaults(cfg *kubeadmapi.JoinConfiguration) error {
-	cfg.NodeRegistration.Name = node.GetHostname(cfg.NodeRegistration.Name)
+	nodeName, err := node.GetHostname(cfg.NodeRegistration.Name)
+	if err != nil {
+		return err
+	}
+	cfg.NodeRegistration.Name = nodeName
 	return nil
 }
 

cmd/kubelet/app/server.go

@@ -527,7 +527,11 @@ func run(s *options.KubeletServer, kubeDeps *kubelet.Dependencies, stopCh <-chan
 		}
 	}
 
-	nodeName, err := getNodeName(kubeDeps.Cloud, nodeutil.GetHostname(s.HostnameOverride))
+	hostName, err := nodeutil.GetHostname(s.HostnameOverride)
+	if err != nil {
+		return err
+	}
+	nodeName, err := getNodeName(kubeDeps.Cloud, hostName)
 	if err != nil {
 		return err
 	}
@@ -784,7 +788,11 @@ func InitializeTLS(kf *options.KubeletFlags, kc *kubeletconfiginternal.KubeletCo
 			return nil, err
 		}
 		if !canReadCertAndKey {
-			cert, key, err := certutil.GenerateSelfSignedCertKey(nodeutil.GetHostname(kf.HostnameOverride), nil, nil)
+			hostName, err := nodeutil.GetHostname(kf.HostnameOverride)
+			if err != nil {
+				return nil, err
+			}
+			cert, key, err := certutil.GenerateSelfSignedCertKey(hostName, nil, nil)
 			if err != nil {
 				return nil, fmt.Errorf("unable to generate self signed cert: %v", err)
 			}
@@ -886,7 +894,10 @@ func addChaosToClientConfig(s *options.KubeletServer, config *restclient.Config)
 //   3 Standalone 'kubernetes' binary
 // Eventually, #2 will be replaced with instances of #3
 func RunKubelet(kubeServer *options.KubeletServer, kubeDeps *kubelet.Dependencies, runOnce bool) error {
-	hostname := nodeutil.GetHostname(kubeServer.HostnameOverride)
+	hostname, err := nodeutil.GetHostname(kubeServer.HostnameOverride)
+	if err != nil {
+		return err
+	}
 	// Query the cloud provider for our node name, default to hostname if kubeDeps.Cloud == nil
 	nodeName, err := getNodeName(kubeDeps.Cloud, hostname)
 	if err != nil {

pkg/kubelet/kubelet.go

@@ -369,7 +369,10 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 		}
 	}
 
-	hostname := nodeutil.GetHostname(hostnameOverride)
+	hostname, err := nodeutil.GetHostname(hostnameOverride)
+	if err != nil {
+		return nil, err
+	}
 	// Query the cloud provider for our node name, default to hostname
 	nodeName := types.NodeName(hostname)
 	if kubeDeps.Cloud != nil {

pkg/util/node/BUILD

@@ -19,7 +19,6 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
-        "//vendor/github.com/golang/glog:go_default_library",
     ],
 )
 

pkg/util/node/node.go

@@ -24,7 +24,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/golang/glog"
 	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -43,16 +42,23 @@ const (
 )
 
 // GetHostname returns OS's hostname if 'hostnameOverride' is empty; otherwise, return 'hostnameOverride'.
-func GetHostname(hostnameOverride string) string {
-	hostname := hostnameOverride
-	if hostname == "" {
-		nodename, err := os.Hostname()
+func GetHostname(hostnameOverride string) (string, error) {
+	hostName := hostnameOverride
+	if len(hostName) == 0 {
+		nodeName, err := os.Hostname()
 		if err != nil {
-			glog.Fatalf("Couldn't determine hostname: %v", err)
+			return "", fmt.Errorf("couldn't determine hostname: %v", err)
 		}
-		hostname = nodename
+		hostName = nodeName
 	}
-	return strings.ToLower(strings.TrimSpace(hostname))
+
+	// Trim whitespaces first to avoid getting an empty hostname
+	// For linux, the hostname is read from file /proc/sys/kernel/hostname directly
+	hostName = strings.TrimSpace(hostName)
+	if len(hostName) == 0 {
+		return "", fmt.Errorf("empty hostname is invalid")
+	}
+	return strings.ToLower(hostName), nil
 }
 
 // GetPreferredNodeAddress returns the address of the provided node, using the provided preference order.

pkg/util/node/node_test.go

@@ -89,3 +89,35 @@ func TestGetPreferredAddress(t *testing.T) {
 		}
 	}
 }
+
+func TestGetHostname(t *testing.T) {
+	testCases := []struct {
+		hostName         string
+		expectedHostName string
+		expectError      bool
+	}{
+		{
+			hostName:    "   ",
+			expectError: true,
+		},
+		{
+			hostName:         " abc  ",
+			expectedHostName: "abc",
+			expectError:      false,
+		},
+	}
+
+	for idx, test := range testCases {
+		hostName, err := GetHostname(test.hostName)
+		if err != nil && !test.expectError {
+			t.Errorf("[%d]: unexpected error: %s", idx, err)
+		}
+		if err == nil && test.expectError {
+			t.Errorf("[%d]: expected error, got none", idx)
+		}
+		if test.expectedHostName != hostName {
+			t.Errorf("[%d]: expected output %q, got %q", idx, test.expectedHostName, hostName)
+		}
+
+	}
+}

pkg/volume/rbd/rbd_util.go

@@ -318,7 +318,11 @@ func (util *RBDUtil) rbdUnlock(b rbdMounter) error {
 	}
 
 	// Construct lock id using host name and a magic prefix.
-	lock_id := kubeLockMagic + node.GetHostname("")
+	hostName, err := node.GetHostname("")
+	if err != nil {
+		return err
+	}
+	lock_id := kubeLockMagic + hostName
 
 	mon := util.kernelRBDMonitorsOpt(b.Mon)