feat: allow member agent use OS's root certificate authority (follow …

…up) (Azure#365) User OS.lookup to differentiate env not set vs set as empty. Block both env set case.
michaelawyu · May 30, 2023 · 6107234 · 6107234
1 parent b557353
commit 6107234
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 20 deletions.
diff --git a/cmd/memberagent/main.go b/cmd/memberagent/main.go
@@ -116,11 +116,11 @@ func main() {
 	}
 }
 
-func buildHubConfig(hubURL string, useCAAuth bool, tlsClientInsecure bool) (*rest.Config, error) {
+func buildHubConfig(hubURL string, useCertificateAuth bool, tlsClientInsecure bool) (*rest.Config, error) {
 	var hubConfig = &rest.Config{
 		Host: hubURL,
 	}
-	if useCAAuth {
+	if useCertificateAuth {
 		keyFilePath := os.Getenv("IDENTITY_KEY")
 		certFilePath := os.Getenv("IDENTITY_CERT")
 		if keyFilePath == "" {
@@ -160,9 +160,27 @@ func buildHubConfig(hubURL string, useCAAuth bool, tlsClientInsecure bool) (*res
 
 	hubConfig.TLSClientConfig.Insecure = tlsClientInsecure
 	if !tlsClientInsecure {
-		hubConfig.TLSClientConfig.CAFile = os.Getenv("CA_BUNDLE")
-		hubCA := os.Getenv("HUB_CERTIFICATE_AUTHORITY")
-		if hubCA != "" {
+		caBundle, ok := os.LookupEnv("CA_BUNDLE")
+		if ok && caBundle == "" {
+			err := errors.New("environment variable CA_BUNDLE should not be empty")
+			klog.ErrorS(err, "failed to validate system variables")
+			return nil, err
+		}
+		hubCA, ok := os.LookupEnv("HUB_CERTIFICATE_AUTHORITY")
+		if ok && hubCA == "" {
+			err := errors.New("environment variable HUB_CERTIFICATE_AUTHORITY should not be empty")
+			klog.ErrorS(err, "failed to validate system variables")
+			return nil, err
+		}
+		if caBundle != "" && hubCA != "" {
+			err := errors.New("environment variables CA_BUNDLE and HUB_CERTIFICATE_AUTHORITY should not be set at same time")
+			klog.ErrorS(err, "failed to validate system variables")
+			return nil, err
+		}
+
+		if caBundle != "" {
+			hubConfig.TLSClientConfig.CAFile = caBundle
+		} else if hubCA != "" {
 			caData, err := base64.StdEncoding.DecodeString(hubCA)
 			if err != nil {
 				klog.ErrorS(err, "cannot decode hub cluster certificate authority data")
@@ -171,7 +189,6 @@ func buildHubConfig(hubURL string, useCAAuth bool, tlsClientInsecure bool) (*res
 			hubConfig.TLSClientConfig.CAData = caData
 		}
 	}
-
 	return hubConfig, nil
 }
 

diff --git a/cmd/memberagent/main_test.go b/cmd/memberagent/main_test.go
@@ -36,6 +36,14 @@ func Test_buildHubConfig(t *testing.T) {
 			},
 		}, *config)
 	})
+	t.Run("empty CA bundle - error", func(t *testing.T) {
+		t.Setenv("IDENTITY_KEY", "/path/to/key")
+		t.Setenv("IDENTITY_CERT", "/path/to/cert")
+		t.Setenv("CA_BUNDLE", "")
+		config, err := buildHubConfig("https://hub.domain.com", true, false)
+		assert.Nil(t, config)
+		assert.NotNil(t, err)
+	})
 	t.Run("use CA bundle - success", func(t *testing.T) {
 		t.Setenv("IDENTITY_KEY", "/path/to/key")
 		t.Setenv("IDENTITY_CERT", "/path/to/cert")
@@ -52,6 +60,35 @@ func Test_buildHubConfig(t *testing.T) {
 			},
 		}, *config)
 	})
+	t.Run("use CA data - success", func(t *testing.T) {
+		t.Setenv("CONFIG_PATH", "./testdata/token")
+		t.Setenv("HUB_CERTIFICATE_AUTHORITY", "dGhpcyBpcyBhIGZha2UgY2E=")
+		config, err := buildHubConfig("https://hub.domain.com", false, false)
+		assert.NotNil(t, config)
+		assert.Nil(t, err)
+		assert.Equal(t, rest.Config{
+			Host:            "https://hub.domain.com",
+			BearerTokenFile: "./testdata/token",
+			TLSClientConfig: rest.TLSClientConfig{
+				CAData: []byte("this is a fake ca"),
+			},
+		}, *config)
+	})
+	t.Run("empty CA data - error", func(t *testing.T) {
+		t.Setenv("CONFIG_PATH", "./testdata/token")
+		t.Setenv("HUB_CERTIFICATE_AUTHORITY", "")
+		config, err := buildHubConfig("https://hub.domain.com", false, false)
+		assert.Nil(t, config)
+		assert.NotNil(t, err)
+	})
+	t.Run("both of CA bundle and CA data present - error", func(t *testing.T) {
+		t.Setenv("CONFIG_PATH", "./testdata/token")
+		t.Setenv("HUB_CERTIFICATE_AUTHORITY", "dGhpcyBpcyBhIGZha2UgY2E=")
+		t.Setenv("CA_BUNDLE", "/path/to/ca/bundle")
+		config, err := buildHubConfig("https://hub.domain.com", false, false)
+		assert.Nil(t, config)
+		assert.NotNil(t, err)
+	})
 	t.Run("use token auth, no toke path - error", func(t *testing.T) {
 		t.Setenv("CONFIG_PATH", "")
 		config, err := buildHubConfig("https://hub.domain.com", false, false)
@@ -74,20 +111,6 @@ func Test_buildHubConfig(t *testing.T) {
 			BearerTokenFile: "./testdata/token",
 		}, *config)
 	})
-	t.Run("use hub ca data - success", func(t *testing.T) {
-		t.Setenv("CONFIG_PATH", "./testdata/token")
-		t.Setenv("HUB_CERTIFICATE_AUTHORITY", "dGhpcyBpcyBhIGZha2UgY2E=")
-		config, err := buildHubConfig("https://hub.domain.com", false, false)
-		assert.NotNil(t, config)
-		assert.Nil(t, err)
-		assert.Equal(t, rest.Config{
-			Host:            "https://hub.domain.com",
-			BearerTokenFile: "./testdata/token",
-			TLSClientConfig: rest.TLSClientConfig{
-				CAData: []byte("this is a fake ca"),
-			},
-		}, *config)
-	})
 	t.Run("No CA bundle, no Hub CA, not insecure - success", func(t *testing.T) {
 		t.Setenv("CONFIG_PATH", "./testdata/token")
 		config, err := buildHubConfig("https://hub.domain.com", false, false)