Add missing Manifest validations to EnsureComplete

The EnsureComplete method needs to perform various validations on a Config object. This commit adds the missing validations for Manifests. Change-Id: Ib562a67eefbbf19eb64444c4a811d946a9c8e170
2020-01-17 09:30:28 -06:00 · 2020-01-17 09:30:28 -06:00 · f0a14cb4fd
commit f0a14cb4fd
parent 7089223607
2 changed files with 146 additions and 39 deletions
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@ -269,31 +269,59 @@ func (c *Config) reconcileCurrentContext() {
 	}
 }

-// This is called by users of the config to make sure that they have
-// A complete configuration before they try to use it.
-// What is a Complete configuration:
-// Should be :
-//	At least 1 cluster defined
-//	At least 1 authinfo (user) defined
-//	At least 1 context defined
-//	The current context properly associated with an existsing context
-// 	At least one Manifest defined
-//
+// EnsureComplete verifies that a Config object is ready to use.
+// A complete Config object meets the following criteria:
+//   * At least 1 Cluster is defined
+//   * At least 1 AuthInfo (user) is defined
+//   * At least 1 Context is defined
+//   * At least 1 Manifest is defined
+//   * The CurrentContext is set
+//   * The CurrentContext identifies an existing Context
+//   * The CurrentContext identifies an existing Manifest
 func (c *Config) EnsureComplete() error {
 	if len(c.Clusters) == 0 {
-		return errors.New("Config: At least one cluster needs to be defined")
+		return ErrMissingConfig{
+			What: "At least one cluster needs to be defined",
+		}
 	}
+
 	if len(c.AuthInfos) == 0 {
-		return errors.New("Config: At least one Authentication Information (User) needs to be defined")
+		return ErrMissingConfig{
+			What: "At least one Authentication Information (User) needs to be defined",
+		}
 	}

 	if len(c.Contexts) == 0 {
-		return errors.New("Config: At least one Context needs to be defined")
+		return ErrMissingConfig{
+			What: "At least one Context needs to be defined",
+		}
 	}

-	if c.CurrentContext == "" || c.Contexts[c.CurrentContext] == nil {
-		return errors.New("Config: Current Context is not defined, or it doesnt identify a defined Context")
+	if len(c.Manifests) == 0 {
+		return ErrMissingConfig{
+			What: "At least one Manifest needs to be defined",
+		}
 	}
+
+	if c.CurrentContext == "" {
+		return ErrMissingConfig{
+			What: "Current Context is not defined",
+		}
+	}
+
+	currentContext, found := c.Contexts[c.CurrentContext]
+	if !found {
+		return ErrMissingConfig{
+			What: fmt.Sprintf("Current Context (%s) does not identify a defined Context", c.CurrentContext),
+		}
+	}
+
+	if _, found := c.Manifests[currentContext.Manifest]; !found {
+		return ErrMissingConfig{
+			What: fmt.Sprintf("Current Context (%s) does not identify a defined Manifest", c.CurrentContext),
+		}
+	}
+
 	return nil
 }

--- a/pkg/config/config_test.go
+++ b/pkg/config/config_test.go
@ -229,33 +229,112 @@ func TestPersistConfig(t *testing.T) {
 }

 func TestEnsureComplete(t *testing.T) {
-	conf := InitConfig(t)
-
-	conf.CurrentContext = "def_ephemeral"
-	assert.NoError(t, conf.EnsureComplete())
-
-	// Trigger no CurrentContext Error
-	conf.CurrentContext = ""
-	err := conf.EnsureComplete()
-	assert.EqualError(t, err, "Config: Current Context is not defined, or it doesnt identify a defined Context")
-
-	// Trigger Contexts Error
-	for key := range conf.Contexts {
-		delete(conf.Contexts, key)
+	// This test is intentionally verbose. Since a user of EnsureComplete
+	// does not need to know about the order of validation, each test
+	// object passed into EnsureComplete should have exactly one issue, and
+	// be otherwise valid
+	tests := []struct {
+		name        string
+		config      Config
+		expectedErr error
+	}{
+		{
+			name: "no clusters defined",
+			config: Config{
+				Clusters:       map[string]*ClusterPurpose{},
+				AuthInfos:      map[string]*AuthInfo{"testAuthInfo": {}},
+				Contexts:       map[string]*Context{"testContext": {Manifest: "testManifest"}},
+				Manifests:      map[string]*Manifest{"testManifest": {}},
+				CurrentContext: "testContext",
+			},
+			expectedErr: ErrMissingConfig{What: "At least one cluster needs to be defined"},
+		},
+		{
+			name: "no users defined",
+			config: Config{
+				Clusters:       map[string]*ClusterPurpose{"testCluster": {}},
+				AuthInfos:      map[string]*AuthInfo{},
+				Contexts:       map[string]*Context{"testContext": {Manifest: "testManifest"}},
+				Manifests:      map[string]*Manifest{"testManifest": {}},
+				CurrentContext: "testContext",
+			},
+			expectedErr: ErrMissingConfig{What: "At least one Authentication Information (User) needs to be defined"},
+		},
+		{
+			name: "no contexts defined",
+			config: Config{
+				Clusters:       map[string]*ClusterPurpose{"testCluster": {}},
+				AuthInfos:      map[string]*AuthInfo{"testAuthInfo": {}},
+				Contexts:       map[string]*Context{},
+				Manifests:      map[string]*Manifest{"testManifest": {}},
+				CurrentContext: "testContext",
+			},
+			expectedErr: ErrMissingConfig{What: "At least one Context needs to be defined"},
+		},
+		{
+			name: "no manifests defined",
+			config: Config{
+				Clusters:       map[string]*ClusterPurpose{"testCluster": {}},
+				AuthInfos:      map[string]*AuthInfo{"testAuthInfo": {}},
+				Contexts:       map[string]*Context{"testContext": {Manifest: "testManifest"}},
+				Manifests:      map[string]*Manifest{},
+				CurrentContext: "testContext",
+			},
+			expectedErr: ErrMissingConfig{What: "At least one Manifest needs to be defined"},
+		},
+		{
+			name: "current context not defined",
+			config: Config{
+				Clusters:       map[string]*ClusterPurpose{"testCluster": {}},
+				AuthInfos:      map[string]*AuthInfo{"testAuthInfo": {}},
+				Contexts:       map[string]*Context{"testContext": {Manifest: "testManifest"}},
+				Manifests:      map[string]*Manifest{"testManifest": {}},
+				CurrentContext: "",
+			},
+			expectedErr: ErrMissingConfig{What: "Current Context is not defined"},
+		},
+		{
+			name: "no context for current context",
+			config: Config{
+				Clusters:       map[string]*ClusterPurpose{"testCluster": {}},
+				AuthInfos:      map[string]*AuthInfo{"testAuthInfo": {}},
+				Contexts:       map[string]*Context{"DIFFERENT_CONTEXT": {Manifest: "testManifest"}},
+				Manifests:      map[string]*Manifest{"testManifest": {}},
+				CurrentContext: "testContext",
+			},
+			expectedErr: ErrMissingConfig{What: "Current Context (testContext) does not identify a defined Context"},
+		},
+		{
+			name: "no manifest for current context",
+			config: Config{
+				Clusters:       map[string]*ClusterPurpose{"testCluster": {}},
+				AuthInfos:      map[string]*AuthInfo{"testAuthInfo": {}},
+				Contexts:       map[string]*Context{"testContext": {Manifest: "testManifest"}},
+				Manifests:      map[string]*Manifest{"DIFFERENT_MANIFEST": {}},
+				CurrentContext: "testContext",
+			},
+			expectedErr: ErrMissingConfig{What: "Current Context (testContext) does not identify a defined Manifest"},
+		},
+		{
+			name: "complete config",
+			config: Config{
+				Clusters:       map[string]*ClusterPurpose{"testCluster": {}},
+				AuthInfos:      map[string]*AuthInfo{"testAuthInfo": {}},
+				Contexts:       map[string]*Context{"testContext": {Manifest: "testManifest"}},
+				Manifests:      map[string]*Manifest{"testManifest": {}},
+				CurrentContext: "testContext",
+			},
+			expectedErr: nil,
+		},
 	}
-	err = conf.EnsureComplete()
-	assert.EqualError(t, err, "Config: At least one Context needs to be defined")

-	// Trigger Authentication Information
-	for key := range conf.AuthInfos {
-		delete(conf.AuthInfos, key)
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(subTest *testing.T) {
+			actualErr := tt.config.EnsureComplete()
+			assert.Equal(subTest, tt.expectedErr, actualErr)
+		})
 	}
-	err = conf.EnsureComplete()
-	assert.EqualError(t, err, "Config: At least one Authentication Information (User) needs to be defined")
-
-	conf = NewConfig()
-	err = conf.EnsureComplete()
-	assert.Error(t, err, "A new config object should not be complete")
 }

 func TestPurge(t *testing.T) {