From 84ae53dd9ed843c577994bea46bf8667fb255cad Mon Sep 17 00:00:00 2001 From: Lachezar Tsonov <80523253+ltsonov-cb@users.noreply.github.com> Date: Wed, 27 Sep 2023 11:44:34 +0300 Subject: [PATCH] Implement integration with remote configuration APIs and adjust functionality (#186) * Remove feature flags from configuration changes based on changed product requirements * Implement api methods for configuration changes * Delete redundant file * Add mutex to prevent multiple iterations of the configuration to run by mistake * Extract validator creation. If the operator is not a valid semantic version, skip validating compatibility. --- .../communication/gateway/api_gateway.go | 56 +++++- .../gateway/dummy_configuration_data.go | 63 ------ .../models/remote_configuration_changes.go | 12 +- .../remote_configuration/change_applier.go | 18 -- .../change_applier_test.go | 175 +--------------- .../remote_configuration/configurator.go | 23 ++- .../remote_configuration/configurator_test.go | 56 ++---- .../remote_configuration/mocks/generated.go | 5 +- .../mocks/mock_access_token_provider.go | 2 +- .../mocks/mock_api_gateway.go | 2 +- .../mocks/mock_validator.go | 50 +++++ .../remote_configuration/validation.go | 79 ++------ .../remote_configuration/validation_test.go | 189 ------------------ main.go | 15 +- 14 files changed, 179 insertions(+), 566 deletions(-) delete mode 100644 cbcontainers/communication/gateway/dummy_configuration_data.go create mode 100644 cbcontainers/remote_configuration/mocks/mock_validator.go diff --git a/cbcontainers/communication/gateway/api_gateway.go b/cbcontainers/communication/gateway/api_gateway.go index cd4527c0..d22b73a3 100644 --- a/cbcontainers/communication/gateway/api_gateway.go +++ b/cbcontainers/communication/gateway/api_gateway.go @@ -195,18 +195,60 @@ func (gateway *ApiGateway) GetSensorMetadata() ([]models.SensorMetadata, error) // GetConfigurationChanges returns a list of configuration changes for the cluster func (gateway *ApiGateway) GetConfigurationChanges(ctx context.Context, clusterIdentifier string) ([]models.ConfigurationChange, error) { - // TODO: Real implementation with CNS-2790 - c := randomRemoteConfigChange() - if c != nil { - return []models.ConfigurationChange{*c}, nil + type getChangesResponse struct { + Changes []models.ConfigurationChange `json:"configuration_changes"` + } + + group, name, err := gateway.SplitToGroupAndMember() + if err != nil { + return nil, err + } + url := gateway.baseUrl("management/configuration_changes/clusters/{clusterID}") + resp, err := gateway.baseRequest(). + SetResult(getChangesResponse{}). + SetPathParam("clusterID", clusterIdentifier). + SetQueryParam("cluster_group", group). + SetQueryParam("cluster_name", name). + SetContext(ctx). + Get(url) + if err != nil { + return nil, err + } + if !resp.IsSuccess() { + return nil, fmt.Errorf("failed to get pending configuration changes with status code (%d)", resp.StatusCode()) + } + + r, ok := resp.Result().(*getChangesResponse) + if !ok || r == nil { + return nil, fmt.Errorf("malformed configuration changes response") } - return nil, nil + + return r.Changes, nil } // UpdateConfigurationChangeStatus either acknowledges a remote configuration change applied to the cluster or marks the attempt as a failure -func (gateway *ApiGateway) UpdateConfigurationChangeStatus(context.Context, models.ConfigurationChangeStatusUpdate) error { - // TODO: Real implementation with CNS-2790 +// The cluster group and name are filled by the gateway +func (gateway *ApiGateway) UpdateConfigurationChangeStatus(ctx context.Context, changeStatus models.ConfigurationChangeStatusUpdate) error { + group, name, err := gateway.SplitToGroupAndMember() + if err != nil { + return err + } + changeStatus.ClusterGroup = group + changeStatus.ClusterName = name + + url := gateway.baseUrl("management/configuration_changes/{changeID}/status") + resp, err := gateway.baseRequest(). + SetPathParam("changeID", changeStatus.ID). + SetBody(changeStatus). + Post(url) + + if err != nil { + return err + } + if !resp.IsSuccess() { + return fmt.Errorf("call to update configuration change status failed with status code (%d)", resp.StatusCode()) + } return nil } diff --git a/cbcontainers/communication/gateway/dummy_configuration_data.go b/cbcontainers/communication/gateway/dummy_configuration_data.go deleted file mode 100644 index 62fc391b..00000000 --- a/cbcontainers/communication/gateway/dummy_configuration_data.go +++ /dev/null @@ -1,63 +0,0 @@ -package gateway - -import ( - "github.com/vmware/cbcontainers-operator/cbcontainers/models" - "math/rand" - "strconv" -) - -// TODO: This will be removed once real APIs are implemented for this but it helps try the feature while in development -// API task - CNS-2790 - -var ( - tr = true - fal = false - dummyAgentVersions = []string{"2.12.1", "2.10.0", "2.12.0", "2.11.0", "3.0.0"} -) - -func randomRemoteConfigChange() *models.ConfigurationChange { - csRand, runtimeRand, cndrRand, versionRand, nilRand := rand.Int(), rand.Int(), rand.Int(), rand.Intn(len(dummyAgentVersions)), rand.Int() - - if nilRand%5 == 1 { - return nil - } - - changeVersion := &dummyAgentVersions[versionRand] - - var changeClusterScanning *bool - var changeRuntime *bool - var changeCNDR *bool - - switch csRand % 5 { - case 1, 3: - changeClusterScanning = &tr - case 2, 4: - changeClusterScanning = &fal - default: - changeClusterScanning = nil - } - - switch runtimeRand % 5 { - case 1, 3: - changeRuntime = &tr - case 2, 4: - changeRuntime = &fal - default: - changeRuntime = nil - } - - if changeVersion != nil && *changeVersion == "3.0.0" && cndrRand%2 == 0 { - changeCNDR = &tr - } else { - changeCNDR = &fal - } - - return &models.ConfigurationChange{ - ID: strconv.Itoa(rand.Int()), - AgentVersion: changeVersion, - EnableClusterScanning: changeClusterScanning, - EnableRuntime: changeRuntime, - EnableCNDR: changeCNDR, - Status: models.ChangeStatusPending, - } -} diff --git a/cbcontainers/models/remote_configuration_changes.go b/cbcontainers/models/remote_configuration_changes.go index 54603b6d..91211953 100644 --- a/cbcontainers/models/remote_configuration_changes.go +++ b/cbcontainers/models/remote_configuration_changes.go @@ -9,14 +9,10 @@ var ( ) type ConfigurationChange struct { - ID string `json:"id"` - Status RemoteChangeStatus `json:"status"` - AgentVersion *string `json:"agent_version"` - EnableClusterScanning *bool `json:"enable_cluster_scanning"` - EnableRuntime *bool `json:"enable_runtime"` - EnableCNDR *bool `json:"enable_cndr"` - EnableClusterScanningSecretDetection *bool `json:"enable_cluster_scanning_secret_detection"` - Timestamp string `json:"timestamp"` + ID string `json:"id"` + Status RemoteChangeStatus `json:"status"` + AgentVersion *string `json:"agent_version"` + Timestamp string `json:"timestamp"` } type ConfigurationChangeStatusUpdate struct { diff --git a/cbcontainers/remote_configuration/change_applier.go b/cbcontainers/remote_configuration/change_applier.go index 84c33064..9bec35e1 100644 --- a/cbcontainers/remote_configuration/change_applier.go +++ b/cbcontainers/remote_configuration/change_applier.go @@ -29,22 +29,4 @@ func ApplyConfigChangeToCR(change models.ConfigurationChange, cr *cbcontainersv1 i.Tag = "" } } - if change.EnableClusterScanning != nil { - cr.Spec.Components.ClusterScanning.Enabled = change.EnableClusterScanning - } - - if change.EnableClusterScanningSecretDetection != nil { - cr.Spec.Components.ClusterScanning.ClusterScannerAgent.CLIFlags.EnableSecretDetection = *change.EnableClusterScanningSecretDetection - } - - if change.EnableRuntime != nil { - cr.Spec.Components.RuntimeProtection.Enabled = change.EnableRuntime - } - - if change.EnableCNDR != nil { - if cr.Spec.Components.Cndr == nil { - cr.Spec.Components.Cndr = &cbcontainersv1.CBContainersCndrSpec{} - } - cr.Spec.Components.Cndr.Enabled = change.EnableCNDR - } } diff --git a/cbcontainers/remote_configuration/change_applier_test.go b/cbcontainers/remote_configuration/change_applier_test.go index 25a7e150..9035da52 100644 --- a/cbcontainers/remote_configuration/change_applier_test.go +++ b/cbcontainers/remote_configuration/change_applier_test.go @@ -1,7 +1,6 @@ package remote_configuration_test import ( - "fmt" "github.com/stretchr/testify/assert" cbcontainersv1 "github.com/vmware/cbcontainers-operator/api/v1" "github.com/vmware/cbcontainers-operator/cbcontainers/models" @@ -11,127 +10,6 @@ import ( "testing" ) -func TestFeatureTogglesAreAppliedCorrectly(t *testing.T) { - type appliedChangeTest struct { - name string - change models.ConfigurationChange - initialCR cbcontainersv1.CBContainersAgent - assertFinalCR func(*testing.T, cbcontainersv1.CBContainersAgent) - } - - crVersion := "1.2.3" - - // generateFeatureToggleTestCases produces a set of tests for a single feature toggle in the requested change - // The tests validate if each toggle state (true, false, nil) is applied correctly or ignored when it's not needed against the CR's state (true, false, nil) - generateFeatureToggleTestCases := - func(feature string, - changeFieldChanger func(*models.ConfigurationChange, *bool), - crFieldChanger func(*cbcontainersv1.CBContainersAgent, *bool), - crAsserter func(*testing.T, cbcontainersv1.CBContainersAgent, *bool)) []appliedChangeTest { - - var result []appliedChangeTest - - for _, crState := range []*bool{truePtr, falsePtr, nil} { - crState := crState // Avoid closure issues - - // Validate that each toggle state works (or doesn't do anything when it matches) - for _, changeState := range []*bool{falsePtr, truePtr} { - changeState := changeState // Avoid closure issues - - cr := cbcontainersv1.CBContainersAgent{Spec: cbcontainersv1.CBContainersAgentSpec{Version: crVersion}} - crFieldChanger(&cr, crState) - change := models.ConfigurationChange{} - changeFieldChanger(&change, changeState) - - result = append(result, appliedChangeTest{ - name: fmt.Sprintf("toggle feature (%s) from (%v) to (%v)", feature, prettyPrintBoolPtr(crState), prettyPrintBoolPtr(changeState)), - change: change, - initialCR: cr, - assertFinalCR: func(t *testing.T, agent cbcontainersv1.CBContainersAgent) { - crAsserter(t, agent, changeState) - }, - }) - } - - cr := cbcontainersv1.CBContainersAgent{Spec: cbcontainersv1.CBContainersAgentSpec{Version: crVersion}} - crFieldChanger(&cr, crState) - // Validate that a change with the toggle unset does not modify the CR - result = append(result, appliedChangeTest{ - name: fmt.Sprintf("missing toggle feature (%s) with CR state (%v)", feature, prettyPrintBoolPtr(crState)), - change: models.ConfigurationChange{}, - initialCR: cr, - assertFinalCR: func(t *testing.T, agent cbcontainersv1.CBContainersAgent) { - crAsserter(t, agent, crState) - }, - }) - } - - return result - } - - var testCases []appliedChangeTest - - clusterScannerToggleTestCases := generateFeatureToggleTestCases("cluster scanning", - func(change *models.ConfigurationChange, val *bool) { - change.EnableClusterScanning = val - }, func(agent *cbcontainersv1.CBContainersAgent, val *bool) { - agent.Spec.Components.ClusterScanning.Enabled = val - }, func(t *testing.T, agent cbcontainersv1.CBContainersAgent, b *bool) { - assert.Equal(t, b, agent.Spec.Components.ClusterScanning.Enabled) - }) - - secretDetectionToggleTestCases := generateFeatureToggleTestCases("cluster scanning secret detection", - func(change *models.ConfigurationChange, val *bool) { - change.EnableClusterScanningSecretDetection = val - }, func(agent *cbcontainersv1.CBContainersAgent, val *bool) { - if val == nil { - // Bail out, this value is not valid for the flag - return - } - agent.Spec.Components.ClusterScanning.ClusterScannerAgent.CLIFlags.EnableSecretDetection = *val - }, func(t *testing.T, agent cbcontainersv1.CBContainersAgent, b *bool) { - if b == nil { - // Bail out, this value is not valid for the flag - return - } - assert.Equal(t, *b, agent.Spec.Components.ClusterScanning.ClusterScannerAgent.CLIFlags.EnableSecretDetection) - }) - - runtimeToggleTestCases := generateFeatureToggleTestCases("runtime protection", - func(change *models.ConfigurationChange, val *bool) { - change.EnableRuntime = val - }, func(agent *cbcontainersv1.CBContainersAgent, val *bool) { - agent.Spec.Components.RuntimeProtection.Enabled = val - }, func(t *testing.T, agent cbcontainersv1.CBContainersAgent, b *bool) { - assert.Equal(t, b, agent.Spec.Components.RuntimeProtection.Enabled) - }) - - cndrToggleTestCases := generateFeatureToggleTestCases("CNDR", - func(change *models.ConfigurationChange, val *bool) { - change.EnableCNDR = val - }, func(agent *cbcontainersv1.CBContainersAgent, val *bool) { - if agent.Spec.Components.Cndr == nil { - agent.Spec.Components.Cndr = &cbcontainersv1.CBContainersCndrSpec{} - } - agent.Spec.Components.Cndr.Enabled = val - }, func(t *testing.T, agent cbcontainersv1.CBContainersAgent, b *bool) { - assert.Equal(t, b, agent.Spec.Components.Cndr.Enabled) - }) - - testCases = append(testCases, clusterScannerToggleTestCases...) - testCases = append(testCases, secretDetectionToggleTestCases...) - testCases = append(testCases, runtimeToggleTestCases...) - testCases = append(testCases, cndrToggleTestCases...) - - for _, testCase := range testCases { - testCase := testCase - t.Run(testCase.name, func(t *testing.T) { - remote_configuration.ApplyConfigChangeToCR(testCase.change, &testCase.initialCR) - testCase.assertFinalCR(t, testCase.initialCR) - }) - } -} - func TestVersionIsAppliedCorrectly(t *testing.T) { originalVersion := "my-version-42" newVersion := "new-version" @@ -145,7 +23,7 @@ func TestVersionIsAppliedCorrectly(t *testing.T) { func TestMissingVersionDoesNotModifyCR(t *testing.T) { originalVersion := "my-version-42" cr := cbcontainersv1.CBContainersAgent{Spec: cbcontainersv1.CBContainersAgentSpec{Version: originalVersion}} - change := models.ConfigurationChange{AgentVersion: nil, EnableRuntime: truePtr} + change := models.ConfigurationChange{AgentVersion: nil} remote_configuration.ApplyConfigChangeToCR(change, &cr) assert.Equal(t, originalVersion, cr.Spec.Version) @@ -226,59 +104,16 @@ func TestVersionOverwritesCustomTagsByRemovingThem(t *testing.T) { assert.Empty(t, cr.Spec.Components.Cndr.Sensor.Image.Tag) } -func prettyPrintBoolPtr(v *bool) string { - if v == nil { - return "" - } - return fmt.Sprintf("%t", *v) -} - // randomPendingConfigChange creates a non-empty configuration change with randomly populated fields in pending state // the change is not guaranteed to be 100% valid func randomPendingConfigChange() models.ConfigurationChange { var versions = []string{"2.12.1", "2.10.0", "2.12.0", "2.11.0", "3.0.0"} - csRand, runtimeRand, cndrRand, versionRand := rand.Int(), rand.Int(), rand.Int(), rand.Intn(len(versions)) - - changeVersion := &versions[versionRand] - - var changeClusterScanning *bool - var changeRuntime *bool - var changeCNDR *bool - - switch csRand % 5 { - case 1, 3: - changeClusterScanning = truePtr - case 2, 4: - changeClusterScanning = falsePtr - default: - changeClusterScanning = nil - } - - switch runtimeRand % 5 { - case 1, 3: - changeRuntime = truePtr - case 2, 4: - changeRuntime = falsePtr - default: - changeRuntime = nil - } - - switch cndrRand % 5 { - case 1, 3: - changeCNDR = truePtr - case 2, 4: - changeCNDR = falsePtr - default: - changeCNDR = nil - } + changeVersion := &versions[rand.Intn(len(versions))] return models.ConfigurationChange{ - ID: strconv.Itoa(rand.Int()), - AgentVersion: changeVersion, - EnableClusterScanning: changeClusterScanning, - EnableRuntime: changeRuntime, - EnableCNDR: changeCNDR, - Status: models.ChangeStatusPending, + ID: strconv.Itoa(rand.Int()), + AgentVersion: changeVersion, + Status: models.ChangeStatusPending, } } diff --git a/cbcontainers/remote_configuration/configurator.go b/cbcontainers/remote_configuration/configurator.go index 1cb9272b..96c5e1ec 100644 --- a/cbcontainers/remote_configuration/configurator.go +++ b/cbcontainers/remote_configuration/configurator.go @@ -9,6 +9,7 @@ import ( "github.com/vmware/cbcontainers-operator/cbcontainers/models" "sigs.k8s.io/controller-runtime/pkg/client" "sort" + "sync" "time" ) @@ -28,6 +29,12 @@ type AccessTokenProvider interface { GetCBAccessToken(ctx context.Context, cbContainersCluster *cbcontainersv1.CBContainersAgent, deployedNamespace string) (string, error) } +type ChangeValidator interface { + ValidateChange(change models.ConfigurationChange, cr *cbcontainersv1.CBContainersAgent) error +} + +type ValidatorCreator func(gateway ApiGateway) (ChangeValidator, error) + type ApiCreator func(cbContainersCluster *cbcontainersv1.CBContainersAgent, accessToken string) (ApiGateway, error) type Configurator struct { @@ -35,9 +42,12 @@ type Configurator struct { logger logr.Logger accessTokenProvider AccessTokenProvider apiCreator ApiCreator - operatorVersion string deployedNamespace string clusterIdentifier string + + validatorCreator ValidatorCreator + + mux sync.Mutex } func NewConfigurator( @@ -45,7 +55,7 @@ func NewConfigurator( gatewayCreator ApiCreator, logger logr.Logger, accessTokenProvider AccessTokenProvider, - operatorVersion string, + validatorCreator ValidatorCreator, deployedNamespace string, clusterIdentifier string, ) *Configurator { @@ -54,15 +64,18 @@ func NewConfigurator( logger: logger, apiCreator: gatewayCreator, accessTokenProvider: accessTokenProvider, - operatorVersion: operatorVersion, + validatorCreator: validatorCreator, deployedNamespace: deployedNamespace, clusterIdentifier: clusterIdentifier, + mux: sync.Mutex{}, } } func (configurator *Configurator) RunIteration(ctx context.Context) error { ctx, cancel := context.WithTimeout(ctx, timeoutSingleIteration) defer cancel() + configurator.mux.Lock() + defer configurator.mux.Unlock() configurator.logger.Info("Checking for installed agent...") cr, err := configurator.getCR(ctx) @@ -156,12 +169,12 @@ func (configurator *Configurator) getPendingChange(ctx context.Context, apiGatew } func (configurator *Configurator) applyChangeToCR(ctx context.Context, apiGateway ApiGateway, change models.ConfigurationChange, cr *cbcontainersv1.CBContainersAgent) error { - validator, err := NewConfigurationChangeValidator(configurator.operatorVersion, apiGateway) + validator, err := configurator.validatorCreator(apiGateway) if err != nil { return fmt.Errorf("failed to create configuration change validator; %w", err) } if err := validator.ValidateChange(change, cr); err != nil { - return err + return invalidChangeError{msg: err.Error()} } ApplyConfigChangeToCR(change, cr) return configurator.k8sClient.Update(ctx, cr) diff --git a/cbcontainers/remote_configuration/configurator_test.go b/cbcontainers/remote_configuration/configurator_test.go index b4127102..bc132478 100644 --- a/cbcontainers/remote_configuration/configurator_test.go +++ b/cbcontainers/remote_configuration/configurator_test.go @@ -21,11 +21,11 @@ type configuratorMocks struct { k8sClient *k8sMocks.MockClient apiGateway *mocks.MockApiGateway accessTokenProvider *mocks.MockAccessTokenProvider + validator *mocks.MockChangeValidator - stubAccessToken string - stubOperatorVersion string - stubNamespace string - stubClusterID string + stubAccessToken string + stubNamespace string + stubClusterID string } // setupConfigurator sets up mocks and creates a Configurator instance with those mocks and some dummy data @@ -33,6 +33,7 @@ func setupConfigurator(ctrl *gomock.Controller) (*remote_configuration.Configura k8sClient := k8sMocks.NewMockClient(ctrl) apiGateway := mocks.NewMockApiGateway(ctrl) accessTokenProvider := mocks.NewMockAccessTokenProvider(ctrl) + validator := mocks.NewMockChangeValidator(ctrl) var mockAPIProvider remote_configuration.ApiCreator = func( cbContainersCluster *cbcontainersv1.CBContainersAgent, @@ -40,10 +41,12 @@ func setupConfigurator(ctrl *gomock.Controller) (*remote_configuration.Configura ) (remote_configuration.ApiGateway, error) { return apiGateway, nil } + var mockValidatorProvider remote_configuration.ValidatorCreator = func(gateway remote_configuration.ApiGateway) (remote_configuration.ChangeValidator, error) { + return validator, nil + } namespace := "namespace-name" accessToken := "access-token" - operatorVersion := "1.2.3" clusterID := "1234567" accessTokenProvider.EXPECT().GetCBAccessToken(gomock.Any(), gomock.Any(), namespace).Return(accessToken, nil).AnyTimes() @@ -52,7 +55,7 @@ func setupConfigurator(ctrl *gomock.Controller) (*remote_configuration.Configura mockAPIProvider, logr.Discard(), accessTokenProvider, - operatorVersion, + mockValidatorProvider, namespace, clusterID, ) @@ -61,8 +64,8 @@ func setupConfigurator(ctrl *gomock.Controller) (*remote_configuration.Configura k8sClient: k8sClient, apiGateway: apiGateway, accessTokenProvider: accessTokenProvider, + validator: validator, stubAccessToken: accessToken, - stubOperatorVersion: operatorVersion, stubNamespace: namespace, stubClusterID: clusterID, } @@ -84,7 +87,7 @@ func TestConfigChangeIsAppliedAndAcknowledgedCorrectly(t *testing.T) { configChange.AgentVersion = &expectedAgentVersion setupCRInK8S(mocks.k8sClient, cr) - setupValidCompatibilityData(mocks.apiGateway, expectedAgentVersion, mocks.stubOperatorVersion) + mocks.validator.EXPECT().ValidateChange(configChange, cr).Return(nil) mocks.apiGateway.EXPECT().GetConfigurationChanges(gomock.Any(), mocks.stubClusterID).Return([]models.ConfigurationChange{configChange}, nil) // Setup mock assertions @@ -117,7 +120,6 @@ func TestWhenChangeIsNotApplicableShouldReturnError(t *testing.T) { configurator, mocks := setupConfigurator(ctrl) cr := &cbcontainersv1.CBContainersAgent{} - maxAgentVersionForOperator := "4.0.0" agentVersion := "5.0.0" configChange := randomPendingConfigChange() configChange.AgentVersion = &agentVersion @@ -125,13 +127,7 @@ func TestWhenChangeIsNotApplicableShouldReturnError(t *testing.T) { setupCRInK8S(mocks.k8sClient, cr) mocks.apiGateway.EXPECT().GetConfigurationChanges(gomock.Any(), mocks.stubClusterID).Return([]models.ConfigurationChange{configChange}, nil) - // Setup invalid compatibility; no need to do full verification here - this is what the validator tests are for - // We just want to check that _some_ validation happens - mocks.apiGateway.EXPECT().GetSensorMetadata().Return([]models.SensorMetadata{{Version: agentVersion}}, nil) - mocks.apiGateway.EXPECT().GetCompatibilityMatrixEntryFor(mocks.stubOperatorVersion).Return(&models.OperatorCompatibility{ - MinAgent: models.AgentMinVersionNone, - MaxAgent: models.AgentVersion(maxAgentVersionForOperator), - }, nil) + mocks.validator.EXPECT().ValidateChange(configChange, cr).Return(errors.New("some error")) // Setup mock assertions mocks.apiGateway.EXPECT().UpdateConfigurationChangeStatus(gomock.Any(), gomock.Any()). @@ -139,7 +135,7 @@ func TestWhenChangeIsNotApplicableShouldReturnError(t *testing.T) { assert.Equal(t, configChange.ID, update.ID) assert.Equal(t, models.ChangeStatusFailed, update.Status) assert.NotEmpty(t, update.Error) - assert.NotEmpty(t, update.ErrorReason) + assert.NotEmpty(t, update.ErrorReason) // Should be populated for validation errors assert.Equal(t, int64(0), update.AppliedGeneration) assert.Empty(t, update.AppliedTimestamp) assert.Equal(t, mocks.stubClusterID, update.ClusterIdentifier) @@ -205,8 +201,8 @@ func TestWhenThereAreMultiplePendingChangesTheOldestIsSelected(t *testing.T) { newerChange.Timestamp = time.Now().UTC().Add(-1 * time.Hour).Format(time.RFC3339) setupCRInK8S(mocks.k8sClient, nil) + setupValidatorAcceptAll(mocks.validator) mocks.apiGateway.EXPECT().GetConfigurationChanges(gomock.Any(), mocks.stubClusterID).Return([]models.ConfigurationChange{newerChange, olderChange}, nil) - setupValidCompatibilityData(mocks.apiGateway, expectedVersion, mocks.stubOperatorVersion) setupUpdateCRMock(t, mocks.k8sClient, func(agent *cbcontainersv1.CBContainersAgent) { assert.Equal(t, expectedVersion, agent.Spec.Version) @@ -261,9 +257,9 @@ func TestWhenUpdatingCRFailsChangeIsUpdatedAsFailed(t *testing.T) { configChange := randomPendingConfigChange() - mocks.apiGateway.EXPECT().GetConfigurationChanges(gomock.Any(), mocks.stubClusterID).Return([]models.ConfigurationChange{configChange}, nil) setupCRInK8S(mocks.k8sClient, nil) - setupValidCompatibilityData(mocks.apiGateway, *configChange.AgentVersion, mocks.stubOperatorVersion) + setupValidatorAcceptAll(mocks.validator) + mocks.apiGateway.EXPECT().GetConfigurationChanges(gomock.Any(), mocks.stubClusterID).Return([]models.ConfigurationChange{configChange}, nil) errFromService := errors.New("some error") mocks.k8sClient.EXPECT().Update(gomock.Any(), gomock.Any()).Return(errFromService) @@ -295,7 +291,7 @@ func TestWhenUpdatingStatusToBackendFailsShouldReturnError(t *testing.T) { configChange := randomPendingConfigChange() setupCRInK8S(mocks.k8sClient, nil) - setupValidCompatibilityData(mocks.apiGateway, *configChange.AgentVersion, mocks.stubOperatorVersion) + setupValidatorAcceptAll(mocks.validator) mocks.apiGateway.EXPECT().GetConfigurationChanges(gomock.Any(), mocks.stubClusterID).Return([]models.ConfigurationChange{configChange}, nil) mocks.k8sClient.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) @@ -328,12 +324,13 @@ func TestWhenFeatureIsDisabledInCRNothingHappens(t *testing.T) { configurator, mocks := setupConfigurator(ctrl) + fal := false cr := &cbcontainersv1.CBContainersAgent{ Spec: cbcontainersv1.CBContainersAgentSpec{ Components: cbcontainersv1.CBContainersComponentsSpec{ Settings: cbcontainersv1.CBContainersComponentsSettings{ RemoteConfiguration: &cbcontainersv1.CBContainersRemoteConfigurationSettings{ - EnabledForAgent: falsePtr, + EnabledForAgent: &fal, }, }}, }, @@ -373,17 +370,6 @@ func setupUpdateCRMock(t *testing.T, mock *k8sMocks.MockClient, assert func(*cbc }) } -func setupValidCompatibilityData(mockGateway *mocks.MockApiGateway, sensorVersion, operatorVersion string) { - mockGateway.EXPECT().GetSensorMetadata().Return([]models.SensorMetadata{{ - Version: sensorVersion, - SupportsRuntime: true, - SupportsClusterScanning: true, - SupportsCndr: true, - }}, nil) - - mockGateway.EXPECT().GetCompatibilityMatrixEntryFor(operatorVersion).Return(&models.OperatorCompatibility{ - MinAgent: models.AgentMinVersionNone, - MaxAgent: models.AgentMaxVersionLatest, - }, nil) - +func setupValidatorAcceptAll(validator *mocks.MockChangeValidator) { + validator.EXPECT().ValidateChange(gomock.Any(), gomock.Any()).Return(nil).AnyTimes() } diff --git a/cbcontainers/remote_configuration/mocks/generated.go b/cbcontainers/remote_configuration/mocks/generated.go index d0aef348..ff0fb921 100644 --- a/cbcontainers/remote_configuration/mocks/generated.go +++ b/cbcontainers/remote_configuration/mocks/generated.go @@ -1,4 +1,5 @@ package mocks -//go:generate mockgen -destination mock_api_gateway.go -package mocks github.com/vmware/cbcontainers-operator/remote_configuration ApiGateway -//go:generate mockgen -destination mock_access_token_provider.go -package mocks github.com/vmware/cbcontainers-operator/remote_configuration AccessTokenProvider +//go:generate mockgen -destination mock_api_gateway.go -package mocks github.com/vmware/cbcontainers-operator/cbcontainers/remote_configuration ApiGateway +//go:generate mockgen -destination mock_access_token_provider.go -package mocks github.com/vmware/cbcontainers-operator/cbcontainers/remote_configuration AccessTokenProvider +//go:generate mockgen -destination mock_validator.go -package mocks github.com/vmware/cbcontainers-operator/cbcontainers/remote_configuration ChangeValidator diff --git a/cbcontainers/remote_configuration/mocks/mock_access_token_provider.go b/cbcontainers/remote_configuration/mocks/mock_access_token_provider.go index 2c3af28a..e1e32df1 100644 --- a/cbcontainers/remote_configuration/mocks/mock_access_token_provider.go +++ b/cbcontainers/remote_configuration/mocks/mock_access_token_provider.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/vmware/cbcontainers-operator/remote_configuration (interfaces: AccessTokenProvider) +// Source: github.com/vmware/cbcontainers-operator/cbcontainers/remote_configuration (interfaces: AccessTokenProvider) // Package mocks is a generated GoMock package. package mocks diff --git a/cbcontainers/remote_configuration/mocks/mock_api_gateway.go b/cbcontainers/remote_configuration/mocks/mock_api_gateway.go index 1a2c1386..f58f49a1 100644 --- a/cbcontainers/remote_configuration/mocks/mock_api_gateway.go +++ b/cbcontainers/remote_configuration/mocks/mock_api_gateway.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/vmware/cbcontainers-operator/remote_configuration (interfaces: ApiGateway) +// Source: github.com/vmware/cbcontainers-operator/cbcontainers/remote_configuration (interfaces: ApiGateway) // Package mocks is a generated GoMock package. package mocks diff --git a/cbcontainers/remote_configuration/mocks/mock_validator.go b/cbcontainers/remote_configuration/mocks/mock_validator.go new file mode 100644 index 00000000..7d64f259 --- /dev/null +++ b/cbcontainers/remote_configuration/mocks/mock_validator.go @@ -0,0 +1,50 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/vmware/cbcontainers-operator/cbcontainers/remote_configuration (interfaces: ChangeValidator) + +// Package mocks is a generated GoMock package. +package mocks + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + v1 "github.com/vmware/cbcontainers-operator/api/v1" + models "github.com/vmware/cbcontainers-operator/cbcontainers/models" +) + +// MockChangeValidator is a mock of ChangeValidator interface. +type MockChangeValidator struct { + ctrl *gomock.Controller + recorder *MockChangeValidatorMockRecorder +} + +// MockChangeValidatorMockRecorder is the mock recorder for MockChangeValidator. +type MockChangeValidatorMockRecorder struct { + mock *MockChangeValidator +} + +// NewMockChangeValidator creates a new mock instance. +func NewMockChangeValidator(ctrl *gomock.Controller) *MockChangeValidator { + mock := &MockChangeValidator{ctrl: ctrl} + mock.recorder = &MockChangeValidatorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockChangeValidator) EXPECT() *MockChangeValidatorMockRecorder { + return m.recorder +} + +// ValidateChange mocks base method. +func (m *MockChangeValidator) ValidateChange(arg0 models.ConfigurationChange, arg1 *v1.CBContainersAgent) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ValidateChange", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// ValidateChange indicates an expected call of ValidateChange. +func (mr *MockChangeValidatorMockRecorder) ValidateChange(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidateChange", reflect.TypeOf((*MockChangeValidator)(nil).ValidateChange), arg0, arg1) +} diff --git a/cbcontainers/remote_configuration/validation.go b/cbcontainers/remote_configuration/validation.go index 09276d1c..59735b7a 100644 --- a/cbcontainers/remote_configuration/validation.go +++ b/cbcontainers/remote_configuration/validation.go @@ -14,6 +14,18 @@ func (i invalidChangeError) Error() string { return i.msg } +// EmptyConfigurationChangeValidator can be used when no validation should occur +type EmptyConfigurationChangeValidator struct { +} + +func (emptyValidator *EmptyConfigurationChangeValidator) ValidateChange(change models.ConfigurationChange, cr *cbcontainersv1.CBContainersAgent) error { + return nil +} + +type ConfigurationChangeValidator struct { + OperatorCompatibilityData models.OperatorCompatibility +} + func NewConfigurationChangeValidator(operatorVersion string, api ApiGateway) (*ConfigurationChangeValidator, error) { compatibilityMatrix, err := api.GetCompatibilityMatrixEntryFor(operatorVersion) if err != nil { @@ -23,22 +35,11 @@ func NewConfigurationChangeValidator(operatorVersion string, api ApiGateway) (*C return nil, fmt.Errorf("compatibility matrix API returned no data but no error as well, cannot continue") } - sensors, err := api.GetSensorMetadata() - if err != nil { - return nil, err - } - return &ConfigurationChangeValidator{ - SensorData: sensors, OperatorCompatibilityData: *compatibilityMatrix, }, nil } -type ConfigurationChangeValidator struct { - SensorData []models.SensorMetadata - OperatorCompatibilityData models.OperatorCompatibility -} - func (validator *ConfigurationChangeValidator) ValidateChange(change models.ConfigurationChange, cr *cbcontainersv1.CBContainersAgent) error { var versionToValidate string @@ -50,59 +51,5 @@ func (validator *ConfigurationChangeValidator) ValidateChange(change models.Conf versionToValidate = cr.Spec.Version } - if err := validator.validateOperatorAndSensorVersionCompatibility(versionToValidate); err != nil { - return err - } - - return validator.validateSensorAndFeatureCompatibility(versionToValidate, change) -} - -func (validator *ConfigurationChangeValidator) findMatchingSensor(sensorVersion string) *models.SensorMetadata { - for _, sensor := range validator.SensorData { - if sensor.Version == sensorVersion { - return &sensor - } - } - - return nil -} - -func (validator *ConfigurationChangeValidator) validateOperatorAndSensorVersionCompatibility(sensorVersion string) error { - if err := validator.OperatorCompatibilityData.CheckCompatibility(sensorVersion); err != nil { - return invalidChangeError{msg: err.Error()} - } - return nil -} - -func (validator *ConfigurationChangeValidator) validateSensorAndFeatureCompatibility(targetVersion string, change models.ConfigurationChange) error { - sensor := validator.findMatchingSensor(targetVersion) - if sensor == nil { - return fmt.Errorf("could not find sensor metadata for version %s", targetVersion) - } - - if change.EnableClusterScanning != nil && - *change.EnableClusterScanning == true && - !sensor.SupportsClusterScanning { - return invalidChangeError{msg: fmt.Sprintf("sensor version %s does not support cluster scanning feature", targetVersion)} - } - - if change.EnableClusterScanningSecretDetection != nil && - *change.EnableClusterScanningSecretDetection == true && - !sensor.SupportsClusterScanningSecrets { - return invalidChangeError{msg: fmt.Sprintf("sensor version %s does not support secret detection during cluster scanning feature", targetVersion)} - } - - if change.EnableRuntime != nil && - *change.EnableRuntime == true && - !sensor.SupportsRuntime { - return invalidChangeError{msg: fmt.Sprintf("sensor version %s does not support runtime protection feature", targetVersion)} - } - - if change.EnableCNDR != nil && - *change.EnableCNDR == true && - !sensor.SupportsCndr { - return invalidChangeError{msg: fmt.Sprintf("sensor version %s does not support cloud-native detect and response feature", targetVersion)} - } - - return nil + return validator.OperatorCompatibilityData.CheckCompatibility(versionToValidate) } diff --git a/cbcontainers/remote_configuration/validation_test.go b/cbcontainers/remote_configuration/validation_test.go index fbed9294..68141877 100644 --- a/cbcontainers/remote_configuration/validation_test.go +++ b/cbcontainers/remote_configuration/validation_test.go @@ -2,7 +2,6 @@ package remote_configuration_test import ( "errors" - "fmt" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" cbcontainersv1 "github.com/vmware/cbcontainers-operator/api/v1" @@ -12,13 +11,6 @@ import ( "testing" ) -var ( - trueV = true - truePtr = &trueV - falseV = false - falsePtr = &falseV -) - func TestValidatorConstructorReturnsErrOnFailures(t *testing.T) { expectedOperatorVersion := "5.0.0" @@ -30,21 +22,12 @@ func TestValidatorConstructorReturnsErrOnFailures(t *testing.T) { name: "get compatibility returns err", setupGatewayMock: func(gateway *mocks.MockApiGateway) { gateway.EXPECT().GetCompatibilityMatrixEntryFor(expectedOperatorVersion).Return(nil, errors.New("some error")).AnyTimes() - gateway.EXPECT().GetSensorMetadata().Return([]models.SensorMetadata{}, nil).AnyTimes() - }, - }, - { - name: "get sensor metadata returns err", - setupGatewayMock: func(gateway *mocks.MockApiGateway) { - gateway.EXPECT().GetCompatibilityMatrixEntryFor(expectedOperatorVersion).Return(&models.OperatorCompatibility{}, nil).AnyTimes() - gateway.EXPECT().GetSensorMetadata().Return(nil, errors.New("some error")).AnyTimes() }, }, { name: "get compatibility returns nil", setupGatewayMock: func(gateway *mocks.MockApiGateway) { gateway.EXPECT().GetCompatibilityMatrixEntryFor(expectedOperatorVersion).Return(nil, nil).AnyTimes() - gateway.EXPECT().GetSensorMetadata().Return([]models.SensorMetadata{}, nil).AnyTimes() }, }, } @@ -65,176 +48,6 @@ func TestValidatorConstructorReturnsErrOnFailures(t *testing.T) { } } -func TestValidateFailsIfSensorDoesNotSupportRequestedFeature(t *testing.T) { - testCases := []struct { - name string - change models.ConfigurationChange - sensorMeta models.SensorMetadata - }{ - { - name: "cluster scanning", - change: models.ConfigurationChange{ - EnableClusterScanning: truePtr, - }, - sensorMeta: models.SensorMetadata{ - SupportsClusterScanning: false, - }, - }, - { - name: "cluster scanning secret detection", - change: models.ConfigurationChange{ - EnableClusterScanningSecretDetection: truePtr, - }, - sensorMeta: models.SensorMetadata{ - SupportsClusterScanningSecrets: false, - }, - }, - { - name: "runtime protection", - change: models.ConfigurationChange{ - EnableRuntime: truePtr, - }, - sensorMeta: models.SensorMetadata{ - SupportsRuntime: false, - }, - }, - { - name: "CNDR", - change: models.ConfigurationChange{ - EnableCNDR: truePtr, - }, - sensorMeta: models.SensorMetadata{ - SupportsCndr: false, - }, - }, - } - - for _, tC := range testCases { - version := "dummy-version" - tC.sensorMeta.Version = version - target := remote_configuration.ConfigurationChangeValidator{ - SensorData: []models.SensorMetadata{tC.sensorMeta}, - } - - t.Run(fmt.Sprintf("no version in change, %s not supported by current agent", tC.name), func(t *testing.T) { - tC.change.AgentVersion = nil - cr := &cbcontainersv1.CBContainersAgent{Spec: cbcontainersv1.CBContainersAgentSpec{Version: version}} - - err := target.ValidateChange(tC.change, cr) - - assert.Error(t, err) - }) - - t.Run(fmt.Sprintf("change also applies agent version, %s not supported by that version", tC.name), func(t *testing.T) { - tC.change.AgentVersion = &version - cr := &cbcontainersv1.CBContainersAgent{Spec: cbcontainersv1.CBContainersAgentSpec{Version: "some-other-verson"}} - - err := target.ValidateChange(tC.change, cr) - - assert.Error(t, err) - }) - } -} - -func TestValidateFailsIfSensorIsNotInList(t *testing.T) { - sensorMetaWithoutTargetSensor := []models.SensorMetadata{{ - Version: "1.0.0", - IsLatest: false, - SupportsRuntime: true, - SupportsClusterScanning: true, - SupportsClusterScanningSecrets: true, - SupportsCndr: true, - }} - operatorSupportsAll := models.OperatorCompatibility{ - MinAgent: models.AgentMinVersionNone, - MaxAgent: models.AgentMaxVersionLatest, - } - unknownVersion := "1.2.3" - - validator := remote_configuration.ConfigurationChangeValidator{ - SensorData: sensorMetaWithoutTargetSensor, - OperatorCompatibilityData: operatorSupportsAll, - } - - change := models.ConfigurationChange{ - AgentVersion: &unknownVersion, - } - cr := &cbcontainersv1.CBContainersAgent{} - - assert.Error(t, validator.ValidateChange(change, cr)) -} - -func TestValidateSucceedsIfSensorSupportsRequestedFeature(t *testing.T) { - testCases := []struct { - name string - change models.ConfigurationChange - sensorMeta models.SensorMetadata - }{ - { - name: "cluster scanning", - change: models.ConfigurationChange{ - EnableClusterScanning: truePtr, - }, - sensorMeta: models.SensorMetadata{ - SupportsClusterScanning: true, - }, - }, - { - name: "cluster scanning secret detection", - change: models.ConfigurationChange{ - EnableClusterScanningSecretDetection: truePtr, - }, - sensorMeta: models.SensorMetadata{ - SupportsClusterScanningSecrets: true, - }, - }, - { - name: "runtime protection", - change: models.ConfigurationChange{ - EnableRuntime: truePtr, - }, - sensorMeta: models.SensorMetadata{ - SupportsRuntime: true, - }, - }, - { - name: "CNDR", - change: models.ConfigurationChange{ - EnableCNDR: truePtr, - }, - sensorMeta: models.SensorMetadata{ - SupportsCndr: true, - }, - }, - } - - for _, tC := range testCases { - version := "dummy-version" - tC.sensorMeta.Version = version - target := remote_configuration.ConfigurationChangeValidator{ - SensorData: []models.SensorMetadata{tC.sensorMeta}, - } - - t.Run(fmt.Sprintf("no version in change, %s is supported by current agent", tC.name), func(t *testing.T) { - tC.change.AgentVersion = nil - cr := &cbcontainersv1.CBContainersAgent{Spec: cbcontainersv1.CBContainersAgentSpec{Version: version}} - - err := target.ValidateChange(tC.change, cr) - - assert.NoError(t, err) - }) - - t.Run(fmt.Sprintf("change also applies agent version, %s is supported by that version", tC.name), func(t *testing.T) { - tC.change.AgentVersion = &version - cr := &cbcontainersv1.CBContainersAgent{Spec: cbcontainersv1.CBContainersAgentSpec{Version: "some-other-verson"}} - - err := target.ValidateChange(tC.change, cr) - - assert.NoError(t, err) - }) - } -} - func TestValidateFailsIfSensorAndOperatorAreNotCompatible(t *testing.T) { testCases := []struct { name string @@ -262,7 +75,6 @@ func TestValidateFailsIfSensorAndOperatorAreNotCompatible(t *testing.T) { for _, tC := range testCases { t.Run(tC.name, func(t *testing.T) { target := remote_configuration.ConfigurationChangeValidator{ - SensorData: []models.SensorMetadata{{Version: tC.versionToApply}}, OperatorCompatibilityData: tC.operatorCompatibility, } @@ -318,7 +130,6 @@ func TestValidateSucceedsIfSensorAndOperatorAreCompatible(t *testing.T) { for _, tC := range testCases { t.Run(tC.name, func(t *testing.T) { target := remote_configuration.ConfigurationChangeValidator{ - SensorData: []models.SensorMetadata{{Version: tC.versionToApply}}, OperatorCompatibilityData: tC.operatorCompatibility, } diff --git a/main.go b/main.go index 4a722555..ded68115 100644 --- a/main.go +++ b/main.go @@ -183,6 +183,19 @@ func main() { setupLog.Error(err, "unable to read the running operator's version from environment variable") os.Exit(1) } + + var validatorCreator remote_configuration.ValidatorCreator + if err != nil && errors.Is(err, operator.ErrNotSemVer) { + setupLog.Info(fmt.Sprintf("Detected operator version (%s) is not a semantic version. Compatibility checks for remote configuration will be disabled", operatorVersion)) + validatorCreator = func(_ remote_configuration.ApiGateway) (remote_configuration.ChangeValidator, error) { + return &remote_configuration.EmptyConfigurationChangeValidator{}, nil + } + } else { + validatorCreator = func(gateway remote_configuration.ApiGateway) (remote_configuration.ChangeValidator, error) { + return remote_configuration.NewConfigurationChangeValidator(operatorVersion, gateway) + } + } + var configuratorGatewayCreator remote_configuration.ApiCreator = func(cbContainersCluster *operatorcontainerscarbonblackiov1.CBContainersAgent, accessToken string) (remote_configuration.ApiGateway, error) { return gateway.NewDefaultGatewayCreator().CreateGateway(cbContainersCluster, accessToken) } @@ -192,7 +205,7 @@ func main() { configuratorGatewayCreator, log, operator.NewSecretAccessTokenProvider(k8sClient), - operatorVersion, + validatorCreator, operatorNamespace, clusterIdentifier, )