Przeglądaj źródła

update cloud cost integration functionality and handlers (#3548)

Signed-off-by: nickcurie <ncurie@kubecost.com>
Nick Curie 3 miesięcy temu
rodzic
commit
de81a6a9f7

+ 70 - 5
pkg/cloud/config/controller.go

@@ -194,19 +194,19 @@ func (c *Controller) CreateConfig(conf cloud.KeyedConfig) error {
 
 	statuses, err := c.storage.load()
 	if err != nil {
-		return fmt.Errorf("failed to load statuses")
+		return fmt.Errorf("failed to load statuses: %w", err)
 	}
 	source := ConfigControllerSource
 	key := conf.Key()
 
 	_, ok := statuses.Get(key, source)
 	if ok {
-		return fmt.Errorf("config with key %s from source %s already exist", key, source.String())
+		return fmt.Errorf("config with key %s from source %s already exists", key, source.String())
 	}
 
 	configType, err := ConfigTypeFromConfig(conf)
 	if err != nil {
-		return fmt.Errorf("config did not have recoginzed config: %w", err)
+		return fmt.Errorf("provided config did not have recoginzed config type: %w", err)
 	}
 
 	statuses.Insert(&Status{
@@ -224,8 +224,8 @@ func (c *Controller) CreateConfig(conf cloud.KeyedConfig) error {
 			continue
 		}
 
-		// if active disable
-		if confStat.Active == true {
+		// if active, disable and remove from observers
+		if confStat.Active {
 			confStat.Active = false
 			c.broadcastRemoveConfig(key)
 		}
@@ -240,6 +240,71 @@ func (c *Controller) CreateConfig(conf cloud.KeyedConfig) error {
 	return nil
 }
 
+// UpdateConfig updates an existing config in status with a source of ConfigControllerSource
+// It fails if the provided config does not already exist and if the config to be updated was not created by the config controller
+func (c *Controller) UpdateConfig(conf cloud.KeyedConfig) error {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	err := conf.Validate()
+	if err != nil {
+		return fmt.Errorf("provided configuration was invalid: %w", err)
+	}
+
+	statuses, err := c.storage.load()
+	if err != nil {
+		return fmt.Errorf("failed to load statuses: %w", err)
+	}
+	source := ConfigControllerSource
+	key := conf.Key()
+
+	_, ok := statuses.Get(key, source)
+	// we fail here if we're attempting to update any config that isn't created by the config controller
+	if !ok {
+		return fmt.Errorf("unable to find existing config with key %s and source %s", key, source.String())
+	}
+
+	configType, err := ConfigTypeFromConfig(conf)
+	if err != nil {
+		return fmt.Errorf("config did not have recoginzed config: %w", err)
+	}
+
+	// check for configurations with the same configuration key that are already active
+	// and disable them before inserting the updated config
+	for _, confStat := range statuses.List() {
+		if confStat.Key != key {
+			continue
+		}
+
+		// disable previous active configurations
+		// note that this will not disable configs with the same key that are not created by config controller
+		if confStat.Source == ConfigControllerSource && confStat.Active {
+			confStat.Active = false
+			c.broadcastRemoveConfig(key)
+		}
+
+		if confStat.Source != ConfigControllerSource && confStat.Active {
+			log.Debugf("Detected active config with key %s that was not created by the config controller", key)
+		}
+	}
+
+	statuses.Insert(&Status{
+		Key:        key,
+		Source:     source,
+		Valid:      true,
+		Active:     true,
+		ConfigType: configType,
+		Config:     conf,
+	})
+
+	c.broadcastAddConfig(conf)
+	err = c.storage.save(statuses)
+	if err != nil {
+		return fmt.Errorf("failed to save statues: %w", err)
+	}
+	return nil
+}
+
 // EnableConfig enables a config with the given key and source, and disables any config with a matching key
 func (c *Controller) EnableConfig(key, sourceStr string) error {
 	c.lock.Lock()

+ 34 - 0
pkg/cloud/config/controller_handlers.go

@@ -86,6 +86,40 @@ func (c *Controller) GetAddConfigHandler() func(w http.ResponseWriter, r *http.R
 	}
 }
 
+// GetUpdateConfigHandler creates a handler from a http request which updates an existing integration via the integrationController
+func (c *Controller) GetUpdateConfigHandler() func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
+	// perform basic checks to ensure that the pipeline can be accessed
+	fn := c.cloudCostChecks()
+	if fn != nil {
+		return fn
+	}
+
+	// Return valid handler func
+	return func(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
+		w.Header().Set("Content-Type", "application/json")
+
+		configType := r.URL.Query().Get("type")
+		if configType == "" {
+			http.Error(w, "'type' parameter is required", http.StatusBadRequest)
+			return
+		}
+
+		config, err := ParseConfig(configType, r.Body)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		err = c.UpdateConfig(config)
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+
+		protocol.WriteData(w, fmt.Sprintf("Successfully updated integration with key %s", config.Key()))
+	}
+}
+
 func ParseConfig(configType string, body io.Reader) (cloud.KeyedConfig, error) {
 	buf := new(bytes.Buffer)
 	_, err := buf.ReadFrom(body)

+ 134 - 0
pkg/cloud/config/controller_test.go

@@ -1011,6 +1011,140 @@ func TestIntegrationController_CreateConfig(t *testing.T) {
 
 }
 
+func TestIntegrationController_UpdateConfig(t *testing.T) {
+	testCases := map[string]struct {
+		initial   []*Status
+		expected  []*Status
+		input     cloudconfig.KeyedConfig
+		expectErr bool
+	}{
+		"invalid config": {
+			initial: []*Status{
+				makeStatus(validAthenaConf, true, ConfigControllerSource),
+			},
+			expected: []*Status{
+				makeStatus(validAthenaConf, true, ConfigControllerSource),
+			},
+			input:     invalidAthenaConf,
+			expectErr: true,
+		},
+		"config is not from ConfigControllerSource": {
+			initial: []*Status{
+				makeStatus(validAthenaConf, true, MultiCloudSource),
+			},
+			expected: []*Status{
+				makeStatus(validAthenaConf, true, MultiCloudSource),
+			},
+			input:     validAthenaConf,
+			expectErr: true,
+		},
+		"config does not exist": {
+			initial:   []*Status{},
+			expected:  []*Status{},
+			input:     validAthenaConf,
+			expectErr: true,
+		},
+		"update existing config with modified properties": {
+			initial: []*Status{
+				makeStatus(validAthenaConf, true, ConfigControllerSource),
+			},
+			expected: []*Status{
+				makeStatus(validAthenaConfModifiedProperty, true, ConfigControllerSource),
+			},
+			input:     validAthenaConfModifiedProperty,
+			expectErr: false,
+		},
+		"update existing config with same properties": {
+			initial: []*Status{
+				makeStatus(validAthenaConf, true, ConfigControllerSource),
+			},
+			expected: []*Status{
+				makeStatus(validAthenaConf, true, ConfigControllerSource),
+			},
+			input:     validAthenaConf,
+			expectErr: false,
+		},
+		"update existing config when other source has same key": {
+			initial: []*Status{
+				makeStatus(validAthenaConf, false, MultiCloudSource),
+				makeStatus(validAthenaConf, true, ConfigControllerSource),
+			},
+			expected: []*Status{
+				makeStatus(validAthenaConf, false, MultiCloudSource),
+				makeStatus(validAthenaConfModifiedProperty, true, ConfigControllerSource),
+			},
+			input:     validAthenaConfModifiedProperty,
+			expectErr: false,
+		},
+		"update existing config when multiple other configs exist": {
+			initial: []*Status{
+				makeStatus(validBigQueryConf, true, MultiCloudSource),
+				makeStatus(validAthenaConf, true, ConfigControllerSource),
+			},
+			expected: []*Status{
+				makeStatus(validBigQueryConf, true, MultiCloudSource),
+				makeStatus(validAthenaConfModifiedProperty, true, ConfigControllerSource),
+			},
+			input:     validAthenaConfModifiedProperty,
+			expectErr: false,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			// Test set up and validation
+			initialStatuses, err := buildStatuses(tc.initial)
+			if err != nil {
+				t.Errorf("initial statuses: %s", err.Error())
+			}
+
+			expectedStatuses, err := buildStatuses(tc.expected)
+			if err != nil {
+				t.Errorf("expected statuses: %s", err.Error())
+			}
+
+			tempDir := os.TempDir()
+			path := filepath.Join(tempDir, configFile)
+			defer os.Remove(path)
+
+			storage := &FileControllerStorage{
+				path: path,
+			}
+
+			// Initialize controller
+			icd := &Controller{
+				storage: storage,
+			}
+			err = icd.storage.save(initialStatuses)
+			if err != nil {
+				t.Errorf("failed to save initial statuses: %s", err.Error())
+			}
+
+			// Functionality being tested
+			err = icd.UpdateConfig(tc.input)
+
+			// Test Result
+			if err != nil && !tc.expectErr {
+				t.Errorf("unexpected error when updating config: %s", err.Error())
+			}
+			if err == nil && tc.expectErr {
+				t.Errorf("no error where expected")
+			}
+
+			status, err := icd.storage.load()
+			if err != nil {
+				t.Errorf("failed to load status file: %s", err.Error())
+			}
+
+			err = checkStatuses(status, expectedStatuses)
+			if err != nil {
+				t.Errorf("statuses equality check failed: %s", err.Error())
+			}
+		})
+	}
+
+}
+
 func TestIntegrationController_EnableConfig(t *testing.T) {
 	testCases := map[string]struct {
 		initial     []*Status

+ 1 - 1
pkg/cloud/config/statuses.go

@@ -33,7 +33,7 @@ func ConfigTypeFromConfig(config cloud.KeyedConfig) (string, error) {
 	case *oracle.UsageApiConfiguration:
 		return UsageApiConfigType, nil
 	}
-	return "", fmt.Errorf("failed to config type for config with key: %s, type %T", config.Key(), config)
+	return "", fmt.Errorf("failed to determine config type for config with key: %s, type %T", config.Key(), config)
 }
 
 type Statuses map[ConfigSource]map[string]*Status