Clean the parse configuration logic

Signed-off-by: Oluwole Fadeyi <[email protected]>

Author: tfadeyi

pkg/agent/config.go

@@ -127,7 +127,7 @@ func (c *Config) Dump() (string, error) {
 	return string(d), nil
 }
 
-func (c *Config) validate() error {
+func (c *Config) validate(isVenafi bool) error {
 	var result *multierror.Error
 
 	// configured for Venafi Cloud
@@ -139,7 +139,7 @@ func (c *Config) validate() error {
 		if _, err := url.Parse(c.VenafiCloud.UploadPath); err != nil {
 			result = multierror.Append(result, fmt.Errorf("upload_path is not a valid URL"))
 		}
-	} else {
+	} else if !isVenafi {
 		if c.OrganizationID == "" {
 			result = multierror.Append(result, fmt.Errorf("organization_id is required"))
 		}
@@ -167,7 +167,7 @@ func (c *Config) validate() error {
 }
 
 // ParseConfig reads config into a struct used to configure running agents
-func ParseConfig(data []byte) (Config, error) {
+func ParseConfig(data []byte, isVenafi bool) (Config, error) {
 	var config Config
 
 	err := yaml.Unmarshal(data, &config)
@@ -186,7 +186,8 @@ func ParseConfig(data []byte) (Config, error) {
 		config.Endpoint.Protocol = "http"
 	}
 
-	if err = config.validate(); err != nil {
+	err = config.validate(isVenafi)
+	if err != nil {
 		return config, err
 	}
 

pkg/agent/config_test.go

@@ -25,7 +25,7 @@ func TestValidConfigLoad(t *testing.T) {
       output-path: "/nothome"
 `
 
-	loadedConfig, err := ParseConfig([]byte(configFileContents))
+	loadedConfig, err := ParseConfig([]byte(configFileContents), false)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -68,7 +68,7 @@ func TestValidConfigWithEndpointLoad(t *testing.T) {
           always-fail: false
 `
 
-	loadedConfig, err := ParseConfig([]byte(configFileContents))
+	loadedConfig, err := ParseConfig([]byte(configFileContents), false)
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
@@ -114,7 +114,7 @@ func TestValidVenafiCloudConfigLoad(t *testing.T) {
         upload_path: "/testing/path"
 `
 
-	loadedConfig, err := ParseConfig([]byte(configFileContents))
+	loadedConfig, err := ParseConfig([]byte(configFileContents), false)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
@@ -149,7 +149,7 @@ func TestValidVenafiCloudConfigLoad(t *testing.T) {
 func TestInvalidConfigError(t *testing.T) {
 	configFileContents := `data-gatherers: "things"`
 
-	_, parseError := ParseConfig([]byte(configFileContents))
+	_, parseError := ParseConfig([]byte(configFileContents), false)
 
 	expectedError := fmt.Errorf("yaml: unmarshal errors:\n  line 1: cannot unmarshal !!str `things` into []agent.DataGatherer")
 
@@ -159,26 +159,35 @@ func TestInvalidConfigError(t *testing.T) {
 }
 
 func TestMissingConfigError(t *testing.T) {
-	_, parseError := ParseConfig([]byte(""))
-
-	if parseError == nil {
-		t.Fatalf("expected error, got nil")
-	}
-
-	expectedErrorLines := []string{
-		"2 errors occurred:",
-		"\t* organization_id is required",
-		"\t* cluster_id is required",
-		"\n",
-	}
-
-	expectedError := strings.Join(expectedErrorLines, "\n")
-
-	gotError := parseError.Error()
-
-	if gotError != expectedError {
-		t.Errorf("\ngot=\n%v\nwant=\n%s\ndiff=\n%s", gotError, expectedError, diff.Diff(gotError, expectedError))
-	}
+	t.Run("fail to parse config if organization_id or cluster_id are missing (venafi-cloud not enabled)", func(t *testing.T) {
+		_, parseError := ParseConfig([]byte(""), false)
+
+		if parseError == nil {
+			t.Fatalf("expected error, got nil")
+		}
+
+		expectedErrorLines := []string{
+			"2 errors occurred:",
+			"\t* organization_id is required",
+			"\t* cluster_id is required",
+			"\n",
+		}
+
+		expectedError := strings.Join(expectedErrorLines, "\n")
+
+		gotError := parseError.Error()
+
+		if gotError != expectedError {
+			t.Errorf("\ngot=\n%v\nwant=\n%s\ndiff=\n%s", gotError, expectedError, diff.Diff(gotError, expectedError))
+		}
+	})
+	t.Run("successfully parse config if organization_id or cluster_id are missing (venafi-cloud is enabled)", func(t *testing.T) {
+		_, parseError := ParseConfig([]byte(""), true)
+
+		if parseError != nil {
+			t.Fatalf("unxexpected error, no error should have occured when parsing configuration: %s", parseError)
+		}
+	})
 }
 
 func TestPartialMissingConfigError(t *testing.T) {
@@ -190,7 +199,7 @@ func TestPartialMissingConfigError(t *testing.T) {
       organization_id: "example"
       cluster_id: "example-cluster"
       data-gatherers:
-        - kind: dummy`))
+        - kind: dummy`), false)
 
 	if parseError == nil {
 		t.Fatalf("expected error, got nil")
@@ -218,7 +227,7 @@ func TestInvalidServerError(t *testing.T) {
       cluster_id: "my_cluster"
       data-gatherers:
         - kind: dummy
-          name: dummy`))
+          name: dummy`), false)
 
 	if parseError == nil {
 		t.Fatalf("expected error, got nil")
@@ -246,7 +255,7 @@ func TestInvalidDataGathered(t *testing.T) {
         path: /api/v1/data
       schedule: "* * * * *"
       data-gatherers:
-        - kind: "foo"`))
+        - kind: "foo"`), false)
 
 	if parseError == nil {
 		t.Fatalf("expected error, got nil")

pkg/agent/run.go

@@ -198,11 +198,8 @@ func getConfiguration() (Config, client.Client) {
 		log.Fatalf("Failed to read config file: %s", err)
 	}
 
-	config, err := ParseConfig(b)
-	switch {
-	case err != nil && VenafiCloudMode && (config.OrganizationID == "" || config.ClusterID == ""):
-	// venafi-cloud does not require the OrganizationID or ClusterID, do not error in case they are missing
-	case err != nil:
+	config, err := ParseConfig(b, VenafiCloudMode)
+	if err != nil {
 		log.Fatalf("Failed to parse config file: %s", err)
 	}