diff --git a/dev/env/manifests/fleet-manager/04-gitops-config.yaml b/dev/env/manifests/fleet-manager/04-gitops-config.yaml index beb87e0aa8..3c7d24c554 100644 --- a/dev/env/manifests/fleet-manager/04-gitops-config.yaml +++ b/dev/env/manifests/fleet-manager/04-gitops-config.yaml @@ -6,14 +6,30 @@ metadata: data: config.yaml: | rhacsOperators: - crd: - baseURL: https://raw.githubusercontent.com/stackrox/stackrox/{{ .GitRef }}/operator/bundle/manifests/ - gitRef: 4.1.1 + + crdUrls: + - https://raw.githubusercontent.com/stackrox/stackrox/4.1.2/operator/bundle/manifests/platform.stackrox.io_securedclusters.yaml + - https://raw.githubusercontent.com/stackrox/stackrox/4.1.2/operator/bundle/manifests/platform.stackrox.io_centrals.yaml + operators: - - gitRef: 4.1.1 - image: "quay.io/rhacs-eng/stackrox-operator:4.1.1" - - gitRef: 4.1.0 + + - deploymentName: "rhacs-operator-4-2-0" + image: "quay.io/rhacs-eng/stackrox-operator:4.2.0" + centralLabelSelector: "rhacs.redhat.com/version-selector=4.2.0" + securedClusterReconcilerEnabled: false + resources: + requests: + cpu: 100m + memory: 200Mi + + - deploymentName: "rhacs-operator-4-1-0" image: "quay.io/rhacs-eng/stackrox-operator:4.1.0" + centralLabelSelector: "rhacs.redhat.com/version-selector=4.1.0" + securedClusterReconcilerEnabled: false + resources: + requests: + cpu: 100m + memory: 200Mi centrals: overrides: # Set label for all centrals to 4.1.0 @@ -21,7 +37,9 @@ data: patch: | metadata: labels: - rhacs.redhat.com/version-selector: "4.1.0" + rhacs.redhat.com/version-selector: "4.2.0" + + # Adjust centrals for development environment - instanceIds: - "*" patch: | diff --git a/e2e/e2e_canary_upgrade_test.go b/e2e/e2e_canary_upgrade_test.go index 9c81d5a87e..0d88bfc88e 100644 --- a/e2e/e2e_canary_upgrade_test.go +++ b/e2e/e2e_canary_upgrade_test.go @@ -3,6 +3,9 @@ package e2e import ( "context" "fmt" + "os" + "strings" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/central/operator" @@ -17,7 +20,6 @@ import ( v1 "k8s.io/api/core/v1" apiErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "os" ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -25,10 +27,17 @@ const ( namespace = "acscs" ) +func operatorConfigForVersion(version string) operator.OperatorConfig { + return operator.OperatorConfig{ + "deploymentName": fmt.Sprintf("rhacs-operator-%s", strings.ReplaceAll(version, ".", "-")), + "image": fmt.Sprintf("quay.io/rhacs-eng/stackrox-operator:%s", version), + "centralLabelSelector": fmt.Sprintf("rhacs.redhat.com/version-selector=%s", version), + } +} + var _ = Describe("Fleetshard-sync Targeted Upgrade", func() { BeforeEach(func() { - if !features.TargetedOperatorUpgrades.Enabled() || !features.StandaloneMode.Enabled() { Skip("Skipping canary upgrade test") } @@ -44,76 +53,64 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", func() { client, err := fleetmanager.NewClient(fleetManagerEndpoint, auth) Expect(err).ToNot(HaveOccurred()) + operatorConfig1 := operatorConfigForVersion("4.1.1") + operatorConfig2 := operatorConfigForVersion("4.1.2") + Describe("should run ACS operators", func() { It("should deploy single operator", func() { ctx := context.Background() - oneOperatorVersionConfig := []operator.OperatorConfig{{ - GitRef: "4.1.1", - Image: "quay.io/rhacs-eng/stackrox-operator:4.1.1", - }} - err := updateOperatorConfig(ctx, oneOperatorVersionConfig) + operatorConfigs := []operator.OperatorConfig{operatorConfig1} + err := updateOperatorConfig(ctx, operatorConfigs) Expect(err).To(BeNil()) + It("should contain one operator deployment", func() { + Eventually(getOperatorDeployments(ctx)). + WithTimeout(waitTimeout). + WithPolling(defaultPolling). + Should(HaveLen(1)) + }) It("should deploy operator with label selector 4.1.1", func() { - Eventually(validateOperatorDeployment(ctx, "4.1.1", "quay.io/rhacs-eng/stackrox-operator:4.1.1")). + Eventually(operatorMatchesConfig(ctx, operatorConfig1)). WithTimeout(waitTimeout). WithPolling(defaultPolling). Should(Succeed()) }) - }) It("should deploy two operators with different versions", func() { ctx := context.Background() - twoOperatorVersionConfig := []operator.OperatorConfig{{ - GitRef: "4.1.1", - Image: "quay.io/rhacs-eng/stackrox-operator:4.1.1", - }, - { - GitRef: "4.1.2", - Image: "quay.io/rhacs-eng/stackrox-operator:4.1.2", - }, - } + operatorConfigs := []operator.OperatorConfig{operatorConfig1, operatorConfig2} - err := updateOperatorConfig(ctx, twoOperatorVersionConfig) + err := updateOperatorConfig(ctx, operatorConfigs) Expect(err).To(BeNil()) - It("should deploy operator with label selector 4.1.1", func() { - Eventually(validateOperatorDeployment(ctx, "4.1.1", "quay.io/rhacs-eng/stackrox-operator:4.1.1")). + It("should contain only two operator deployments", func() { + Eventually(getOperatorDeployments(ctx)). WithTimeout(waitTimeout). WithPolling(defaultPolling). - Should(Succeed()) + Should(HaveLen(2)) }) - It("should deploy operator with label selector 4.1.2", func() { - Eventually(validateOperatorDeployment(ctx, "4.1.2", "quay.io/rhacs-eng/stackrox-operator:4.1.2")). + It("should deploy operator with label selector 4.1.1", func() { + Eventually(operatorMatchesConfig(ctx, operatorConfig1)). WithTimeout(waitTimeout). WithPolling(defaultPolling). Should(Succeed()) }) - It("should contain only two operator deployments", func() { - Eventually(getOperatorDeployments(ctx)). + It("should deploy operator with label selector 4.1.2", func() { + Eventually(operatorMatchesConfig(ctx, operatorConfig2)). WithTimeout(waitTimeout). WithPolling(defaultPolling). - Should(HaveLen(2)) + Should(Succeed()) }) }) It("should delete the removed operator", func() { ctx := context.Background() - operatorConfig := []operator.OperatorConfig{{ - GitRef: "4.1.2", - Image: "quay.io/rhacs-eng/stackrox-operator:4.1.2", - }} - err := updateOperatorConfig(ctx, operatorConfig) + operatorConfigs := []operator.OperatorConfig{operatorConfig1} + err := updateOperatorConfig(ctx, operatorConfigs) Expect(err).To(BeNil()) - It("should deploy operator with label selector 4.1.2", func() { - Eventually(validateOperatorDeployment(ctx, "4.1.2", "quay.io/rhacs-eng/stackrox-operator:4.1.2")). - WithTimeout(waitTimeout). - WithPolling(defaultPolling). - Should(Succeed()) - }) It("should contain only one operator deployments", func() { Eventually(getOperatorDeployments(ctx)). WithTimeout(waitTimeout). @@ -121,8 +118,13 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", func() { Should(HaveLen(1)) }) + It("should deploy operator with label selector 4.1.2", func() { + Eventually(operatorMatchesConfig(ctx, operatorConfig1)). + WithTimeout(waitTimeout). + WithPolling(defaultPolling). + Should(Succeed()) + }) }) - }) Describe("should upgrade the central", func() { @@ -147,10 +149,8 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", func() { centralNamespace, err = services.FormatNamespace(createdCentral.Id) ctx := context.Background() - oneOperatorVersionConfig := []operator.OperatorConfig{{ - GitRef: "4.1.1", - Image: "quay.io/rhacs-eng/stackrox-operator:4.1.1", - }} + operatorConfig1 := operatorConfigForVersion("4.1.1") + oneOperatorVersionConfig := []operator.OperatorConfig{operatorConfig1} err = updateOperatorConfig(ctx, oneOperatorVersionConfig) Expect(err).To(BeNil()) @@ -165,7 +165,7 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", func() { return nil }).WithTimeout(waitTimeout).WithPolling(defaultPolling).Should(Succeed()) Eventually(func() error { - centralDeployment, err := getCentralDeployment(ctx, createdCentral.Name, centralNamespace) + centralDeployment, err := getDeployment(ctx, centralNamespace, createdCentral.Name) if err != nil { return fmt.Errorf("failed finding central deployment: %v", err) } @@ -178,15 +178,14 @@ var _ = Describe("Fleetshard-sync Targeted Upgrade", func() { It("upgrade central", func() { ctx := context.Background() - oneOperatorVersionConfig := []operator.OperatorConfig{{ - GitRef: "4.1.2", - Image: "quay.io/rhacs-eng/stackrox-operator:4.1.2", - }} + oneOperatorVersionConfig := []operator.OperatorConfig{ + operatorConfigForVersion("4.1.2"), + } err = updateOperatorConfig(ctx, oneOperatorVersionConfig) Expect(err).To(BeNil()) Eventually(func() error { - centralDeployment, err := getCentralDeployment(ctx, createdCentral.Name, centralNamespace) + centralDeployment, err := getDeployment(ctx, centralNamespace, createdCentral.Name) if err != nil { return fmt.Errorf("failed finding central deployment: %v", err) } @@ -249,51 +248,70 @@ func getCentralCR(ctx context.Context, name string, namespace string) (*v1alpha1 return central, nil } -func getCentralDeployment(ctx context.Context, name string, namespace string) (*appsv1.Deployment, error) { +func getDeployment(ctx context.Context, namespace string, name string) (*appsv1.Deployment, error) { deployment := &appsv1.Deployment{} - centralKey := ctrlClient.ObjectKey{Namespace: namespace, Name: name} - err := k8sClient.Get(ctx, centralKey, deployment) + err := k8sClient.Get(ctx, ctrlClient.ObjectKey{Namespace: namespace, Name: name}, deployment) return deployment, err } -func validateOperatorDeployment(ctx context.Context, gitRef string, image string) error { - deploymentName := "rhacs-operator-" + gitRef - labelSelectorEnv := "rhacs.redhat.com/version-selector=" + gitRef - deployment := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: deploymentName, - Namespace: operator.ACSOperatorNamespace, - }, +func getContainer(name string, containers []v1.Container) (*v1.Container, error) { + for _, container := range containers { + if container.Name == name { + return &container, nil + } } + return nil, fmt.Errorf("container %s not found", name) +} - err := k8sClient.Get(ctx, ctrlClient.ObjectKey{Namespace: operator.ACSOperatorNamespace, Name: deploymentName}, deployment) - if err != nil { - return err +type operatorAssertion func(deploy *appsv1.Deployment) error + +var operatorHasImage = func(image string) operatorAssertion { + return func(deploy *appsv1.Deployment) error { + container, err := getContainer("manager", deploy.Spec.Template.Spec.Containers) + if err != nil { + return err + } + if container.Image != image { + return fmt.Errorf("incorrect image %s", container.Image) + } + return nil } - foundManager := false - foundLabelSelector := false - containers := deployment.Spec.Template.Spec.Containers - for _, container := range containers { - if container.Name == "manager" { - foundManager = true - if container.Image != image { - return fmt.Errorf("incorrect operator image %s", container.Image) - } - for _, envVar := range container.Env { - if envVar.Name == "CENTRAL_LABEL_SELECTOR" { - foundLabelSelector = true - if envVar.Value != labelSelectorEnv { - return fmt.Errorf("incorrect CENTRAL_LABEL_SELECTOR %s", envVar.Value) - } +} + +var operatorHasCentralLabelSelector = func(labelSelector string) operatorAssertion { + return func(deploy *appsv1.Deployment) error { + container, err := getContainer("manager", deploy.Spec.Template.Spec.Containers) + if err != nil { + return err + } + for _, envVar := range container.Env { + if envVar.Name == "CENTRAL_LABEL_SELECTOR" { + if envVar.Value != labelSelector { + return fmt.Errorf("incorrect value %s", envVar.Value) } } - if !foundLabelSelector { - return fmt.Errorf("environment variable CENTRAL_LABEL_SELECTOR not found") - } } + return nil } - if !foundManager { - return fmt.Errorf("manager container not found") +} + +func validateOperatorDeployment(deployment *appsv1.Deployment, assertions ...operatorAssertion) error { + for _, assertion := range assertions { + err := assertion(deployment) + if err != nil { + return err + } } return nil } + +func operatorMatchesConfig(ctx context.Context, config operator.OperatorConfig) error { + deploy, err := getDeployment(ctx, operator.ACSOperatorNamespace, config.GetDeploymentName()) + if err != nil { + return err + } + return validateOperatorDeployment(deploy, + operatorHasImage(config.GetImage()), + operatorHasCentralLabelSelector(config.GetCentralLabelSelector()), + ) +} diff --git a/fleetshard/pkg/central/charts/charts.go b/fleetshard/pkg/central/charts/charts.go index 7bfa01b74b..273375676e 100644 --- a/fleetshard/pkg/central/charts/charts.go +++ b/fleetshard/pkg/central/charts/charts.go @@ -147,7 +147,7 @@ func InstallOrUpdateChart(ctx context.Context, obj *unstructured.Unstructured, c return fmt.Errorf("failed to retrieve object %s/%s of type %s %w", key.Namespace, key.Name, obj.GetKind(), err) } err = client.Create(ctx, obj) - glog.Infof("Creating object %s/%s", obj.GetNamespace(), obj.GetName()) + glog.Infof("Creating object %s/%s %s/%s", obj.GetAPIVersion(), obj.GetKind(), obj.GetNamespace(), obj.GetName()) if err != nil && !apiErrors.IsAlreadyExists(err) { return fmt.Errorf("failed to create object %s/%s of type %s: %w", key.Namespace, key.Name, obj.GetKind(), err) } diff --git a/fleetshard/pkg/central/charts/data/rhacs-operator/templates/rhacs-operator-deployment.yaml b/fleetshard/pkg/central/charts/data/rhacs-operator/templates/rhacs-operator-deployment.yaml index c506d2d8c1..36df8c689c 100644 --- a/fleetshard/pkg/central/charts/data/rhacs-operator/templates/rhacs-operator-deployment.yaml +++ b/fleetshard/pkg/central/charts/data/rhacs-operator/templates/rhacs-operator-deployment.yaml @@ -82,9 +82,21 @@ spec: containerName: manager resource: limits.memory divisor: '0' - {{- if .labelSelector }} + {{- if .centralLabelSelector }} - name: CENTRAL_LABEL_SELECTOR - value: "rhacs.redhat.com/version-selector={{ .labelSelector }}" + value: "{{ .centralLabelSelector }}" + {{- end }} + {{- if .securedClusterLabelSelector }} + - name: SECURED_CLUSTER_LABEL_SELECTOR + value: "{{ .securedClusterLabelSelector }}" + {{- end }} + {{- if eq .centralReconcilerEnabled false }} + - name: CENTRAL_RECONCILER_ENABLED + value: "false" + {{- end }} + {{- if eq .securedClusterReconcilerEnabled false }} + - name: SECURED_CLUSTER_RECONCILER_ENABLED + value: "false" {{- end }} image: "{{ .image }}" imagePullPolicy: IfNotPresent diff --git a/fleetshard/pkg/central/charts/data/rhacs-operator/values.yaml b/fleetshard/pkg/central/charts/data/rhacs-operator/values.yaml index 5f79e26650..c40b0f137c 100644 --- a/fleetshard/pkg/central/charts/data/rhacs-operator/values.yaml +++ b/fleetshard/pkg/central/charts/data/rhacs-operator/values.yaml @@ -3,15 +3,26 @@ # Declare variables to be passed into your templates. operator: - # Each item in images should contain `deploymentName`, `image` and `labelSelector` keys + # Each item in images should contain `deploymentName`, `image` and `centralLabelSelector` keys # example: # images: - # - deploymentName: rhacs-operator-manager-4.0.0 + # + # - deploymentName: rhacs-operator-manager-4-0-0 # image: quay.io/rhacs-eng/stackrox-operator:4.0.0 - # labelSelector: 4.0.0 - # - deploymentName: rhacs-operator-manager-4.0.1 + # centralLabelSelector: 4.0.0 + # securedClusterLabelSelector: 4.0.0 + # centralReconcilerEnabled: true + # securedClusterReconcilerEnabled: true + # ...other helm values + # + # - deploymentName: rhacs-operator-manager-4-0-1 # image: quay.io/rhacs-eng/stackrox-operator:4.0.1 - # labelSelector: 4.0.1 + # centralLabelSelector: 4.0.1 + # securedClusterLabelSelector: 4.0.0 + # centralReconcilerEnabled: false + # securedClusterReconcilerEnabled: false + # ...other helm values + # images: [] # TODO: add values for resource limits and requests for both proxy and manager container # TODO: add value for kube-rbac-proxy image diff --git a/fleetshard/pkg/central/operator/config.go b/fleetshard/pkg/central/operator/config.go index 51bda1a1b5..202594b303 100644 --- a/fleetshard/pkg/central/operator/config.go +++ b/fleetshard/pkg/central/operator/config.go @@ -3,10 +3,18 @@ package operator import ( "fmt" "github.com/stackrox/acs-fleet-manager/internal/dinosaur/pkg/api/private" - "k8s.io/apimachinery/pkg/util/validation/field" "sigs.k8s.io/yaml" ) +const ( + keyDeploymentName = "deploymentName" + keyImage = "image" + keyCentralReconcilerEnabled = "centralReconcilerEnabled" + keySecuredClusterReconcilerEnabled = "securedClusterReconcilerEnabled" + keyCentralLabelSelector = "centralLabelSelector" + keySecuredClusterSelector = "securedClusterLabelSelector" +) + func parseConfig(content []byte) (OperatorConfigs, error) { var out OperatorConfigs err := yaml.Unmarshal(content, &out) @@ -16,82 +24,76 @@ func parseConfig(content []byte) (OperatorConfigs, error) { return out, nil } -// Validate validates the operator configuration and can be used in different life-cycle stages like runtime and deploy time. -func Validate(path *field.Path, configs OperatorConfigs) field.ErrorList { - manifests, err := RenderChart(configs) - if err != nil { - return field.ErrorList{ - field.Forbidden(path, fmt.Sprintf("could not render operator helm charts, got invalid configuration: %s", err.Error())), - } - } else if len(configs.Configs) > 0 && len(manifests) == 0 { - return field.ErrorList{ - field.Forbidden(path, fmt.Sprintf("operator chart rendering succeed, but no manifests were rendered")), - } - } - return nil -} - -// CRDConfig represents the crd to be installed in the data-plane cluster. The CRD is downloaded automatically -// from the base URL. It takes a GitRef to resolve a GitHub link to the CRD definition. -type CRDConfig struct { - BaseURL string `json:"baseURL,omitempty"` - GitRef string `json:"gitRef"` -} - // OperatorConfigs represents all operators and the CRD which should be installed in a data-plane cluster. type OperatorConfigs struct { - CRD CRDConfig `json:"crd"` + CRDURLs []string `json:"crdUrls"` Configs []OperatorConfig `json:"operators"` } // OperatorConfig represents the configuration of an operator. -type OperatorConfig struct { - Image string `json:"image"` - GitRef string `json:"gitRef"` - HelmValues string `json:"helmValues,omitempty"` +type OperatorConfig map[string]interface{} + +// GetDeploymentName returns the deployment name of the operator. +func (o OperatorConfig) GetDeploymentName() string { + return o.getString(keyDeploymentName) } -// ToAPIResponse transforms the config to an private API response. -func (o OperatorConfigs) ToAPIResponse() private.RhacsOperatorConfigs { - apiConfigs := private.RhacsOperatorConfigs{ - CRD: private.RhacsOperatorConfigsCrd{ - GitRef: o.CRD.GitRef, - BaseURL: o.CRD.BaseURL, - }, - } +// GetImage returns the image of the operator. +func (o OperatorConfig) GetImage() string { + return o.getString(keyImage) +} - for _, config := range o.Configs { - apiConfigs.RHACSOperatorConfigs = append(apiConfigs.RHACSOperatorConfigs, config.ToAPIResponse()) - } - return apiConfigs +// GetCentralLabelSelector returns the central label selector. +func (o OperatorConfig) GetCentralLabelSelector() string { + return o.getString(keyCentralLabelSelector) +} + +// GetSecuredClusterLabelSelector returns the secured cluster label selector. +func (o OperatorConfig) GetSecuredClusterLabelSelector() string { + return o.getString(keySecuredClusterSelector) +} + +// GetCentralReconcilerEnabled returns true if the central reconciler should be disabled. +func (o OperatorConfig) GetCentralReconcilerEnabled() bool { + return o.getBool(keyCentralReconcilerEnabled) +} + +// GetSecuredClusterReconcilerEnabled returns true if the secured cluster reconciler should be disabled. +func (o OperatorConfig) GetSecuredClusterReconcilerEnabled() bool { + return o.getBool(keySecuredClusterReconcilerEnabled) } -// ToAPIResponse converts the internal OperatorConfig to the openapi generated private.RhacsOperatorConfig type. -func (o OperatorConfig) ToAPIResponse() private.RhacsOperatorConfig { - return private.RhacsOperatorConfig{ - Image: o.Image, - GitRef: o.GitRef, - HelmValues: o.HelmValues, +func (o OperatorConfig) getString(key string) string { + valIntf, ok := o[key] + if !ok { + return "" + } + val, ok := valIntf.(string) + if !ok { + return "" } + return val } -// FromAPIResponse converts an openapi generated model to the internal OperatorConfigs type -func FromAPIResponse(config private.RhacsOperatorConfigs) OperatorConfigs { - var operatorConfigs []OperatorConfig - for _, apiConfig := range config.RHACSOperatorConfigs { - config := OperatorConfig{ - Image: apiConfig.Image, - GitRef: apiConfig.GitRef, - HelmValues: apiConfig.HelmValues, - } - operatorConfigs = append(operatorConfigs, config) +func (o OperatorConfig) getBool(key string) bool { + valIntf, ok := o[key] + if !ok { + return false } + val, ok := valIntf.(bool) + if !ok { + return false + } + return val +} - return OperatorConfigs{ - Configs: operatorConfigs, - CRD: CRDConfig{ - GitRef: config.CRD.GitRef, - BaseURL: config.CRD.BaseURL, - }, +// ToAPIResponse transforms the config to an private API response. +func (o OperatorConfigs) ToAPIResponse() private.RhacsOperatorConfigs { + apiConfigs := private.RhacsOperatorConfigs{ + CrdUrls: o.CRDURLs, + } + for _, config := range o.Configs { + apiConfigs.RHACSOperatorConfigs = append(apiConfigs.RHACSOperatorConfigs, config) } + return apiConfigs } diff --git a/fleetshard/pkg/central/operator/config_test.go b/fleetshard/pkg/central/operator/config_test.go index 25678e4464..b7a29145fb 100644 --- a/fleetshard/pkg/central/operator/config_test.go +++ b/fleetshard/pkg/central/operator/config_test.go @@ -3,103 +3,47 @@ package operator import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/util/validation/field" "testing" ) func getExampleConfig() []byte { return []byte(` -crd: - baseURL: https://raw.githubusercontent.com/stackrox/stackrox/{{ .GitRef }}/operator/bundle/manifests/ - gitRef: 4.1.1 +crdUrls: + - https://raw.githubusercontent.com/stackrox/stackrox/4.1.2/operator/bundle/manifests/platform.stackrox.io_securedclusters.yaml + - https://raw.githubusercontent.com/stackrox/stackrox/4.1.2/operator/bundle/manifests/platform.stackrox.io_centrals.yaml operators: -- gitRef: 4.1.1 +- deploymentName: stackrox-operator image: "quay.io/rhacs-eng/stackrox-operator:4.1.1" - helmValues: | - operator: - resources: - requests: - memory: 500Mi - cpu: 50m + labelSelector: "app.kubernetes.io/name=stackrox-operator" + centralLabelSelector: "app.kubernetes.io/name=central" + securedClusterLabelSelector: "app.kubernetes.io/name=securedCluster" + centralReconcilerEnabled: true + securedClusterReconcilerEnabled: true `) } +func validOperatorConfig() OperatorConfig { + return OperatorConfig{ + keyDeploymentName: "stackrox-operator", + keyImage: "quay.io/rhacs-eng/stackrox-operator:4.1.1", + keyCentralLabelSelector: "app.kubernetes.io/name=central", + keySecuredClusterSelector: "app.kubernetes.io/name=securedCluster", + } +} + func TestGetOperatorConfig(t *testing.T) { conf, err := parseConfig(getExampleConfig()) require.NoError(t, err) - assert.Len(t, conf.Configs, 1) - assert.Equal(t, "4.1.1", conf.Configs[0].GitRef) - assert.Equal(t, "quay.io/rhacs-eng/stackrox-operator:4.1.1", conf.Configs[0].Image) -} - -func TestGetOperatorConfigFailsValidation(t *testing.T) { - testCases := map[string]struct { - getConfig func(*testing.T, OperatorConfigs) OperatorConfigs - contains string - success bool - }{ - "should fail with invalid baseURL not able to download CRD": { - getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { - config.CRD.BaseURL = "not an url" - return config - }, - contains: "failed downloading chart files", - }, - "should fail with invalid git ref": { - getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { - config.Configs = []OperatorConfig{ - {GitRef: "%^-invalid", Image: "quay.io/rhacs-eng/test:4.0.0", HelmValues: ""}, - } - return config - }, - contains: "failed to parse images: label selector %^-invalid is not valid", - }, - "should fail with invalid image": { - getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { - config.Configs = []OperatorConfig{ - {GitRef: "4.0.0", Image: "quay.io//invalid", HelmValues: ""}, - } - return config - }, - contains: "failed to parse images: invalid reference format", - }, - "should fail with invalid helm values": { - getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { - config.Configs = []OperatorConfig{ - {GitRef: "4.0.0", Image: "quay.io/rhacs-eng/test:4.0.0", HelmValues: "invalid YAML"}, - } - return config - }, - contains: "Unmarshalling Helm values failed for operator 4.0.0", - }, - "validate should succeed with example config": { - getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { - return config - }, - success: true, - }, - "should succeed with empty operator configs": { - getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { - config.Configs = []OperatorConfig{} - return config - }, - success: true, - }, - } - - for key, testCase := range testCases { - t.Run(key, func(t *testing.T) { - config, err := parseConfig(getExampleConfig()) - require.NoError(t, err) - - errList := Validate(field.NewPath("rhacsOperator"), testCase.getConfig(t, config)) - if testCase.contains != "" { - require.Len(t, errList, 1) - require.NotEmpty(t, testCase.contains) - assert.Contains(t, errList.ToAggregate().Errors()[0].Error(), testCase.contains) - } else { - require.Nil(t, errList) - } - }) - } + assert.Equal(t, []string{ + "https://raw.githubusercontent.com/stackrox/stackrox/4.1.2/operator/bundle/manifests/platform.stackrox.io_securedclusters.yaml", + "https://raw.githubusercontent.com/stackrox/stackrox/4.1.2/operator/bundle/manifests/platform.stackrox.io_centrals.yaml", + }, conf.CRDURLs) + require.Len(t, conf.Configs, 1) + operatorConfig := conf.Configs[0] + assert.Equal(t, "stackrox-operator", operatorConfig.GetDeploymentName()) + assert.Equal(t, "quay.io/rhacs-eng/stackrox-operator:4.1.1", operatorConfig.GetImage()) + assert.Equal(t, "app.kubernetes.io/name=central", operatorConfig.GetCentralLabelSelector()) + assert.Equal(t, "app.kubernetes.io/name=securedCluster", operatorConfig.GetSecuredClusterLabelSelector()) + assert.True(t, operatorConfig.GetCentralReconcilerEnabled()) + assert.True(t, operatorConfig.GetSecuredClusterReconcilerEnabled()) } diff --git a/fleetshard/pkg/central/operator/upgrade.go b/fleetshard/pkg/central/operator/upgrade.go index fdc1c60003..cd9f8a5e43 100644 --- a/fleetshard/pkg/central/operator/upgrade.go +++ b/fleetshard/pkg/central/operator/upgrade.go @@ -4,68 +4,26 @@ package operator import ( "context" "fmt" - containerImage "github.com/containers/image/docker/reference" "github.com/stackrox/acs-fleet-manager/fleetshard/pkg/central/charts" "golang.org/x/exp/slices" "gopkg.in/yaml.v2" "helm.sh/helm/v3/pkg/chartutil" - "html/template" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/util/validation" ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" - "strings" ) const ( // ACSOperatorNamespace default Operator Namespace ACSOperatorNamespace = "rhacs" // ACSOperatorConfigMap name for configMap with operator deployment configurations - ACSOperatorConfigMap = "operator-config" - releaseName = "rhacs-operator" - operatorDeploymentPrefix = "rhacs-operator" - defaultCRDBaseURLTemplate = "https://raw.githubusercontent.com/stackrox/stackrox/{{ .GitRef }}/operator/bundle/manifests/" + ACSOperatorConfigMap = "operator-config" + releaseName = "rhacs-operator" + operatorDeploymentPrefix = "rhacs-operator" ) -func parseOperatorConfigs(operators OperatorConfigs) ([]chartutil.Values, error) { - var helmValues []chartutil.Values - for _, operator := range operators.Configs { - imageReference, err := containerImage.Parse(operator.Image) - if err != nil { - return nil, err - } - image := imageReference.String() - if errs := validation.IsValidLabelValue(operator.GitRef); errs != nil { - return nil, fmt.Errorf("label selector %s is not valid: %v", operator.GitRef, errs) - } - - deploymentName := generateDeploymentName(operator.GitRef) - // validate deploymentName (RFC-1123) - // More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names - if errs := apimachineryvalidation.NameIsDNSSubdomain(deploymentName, true); errs != nil { - return nil, fmt.Errorf("invalid deploymentName %s: %v", deploymentName, errs) - } - operatorValues := chartutil.Values{ - "deploymentName": deploymentName, - "image": image, - "labelSelector": operator.GitRef, - } - - operatorHelmValues := make(map[string]interface{}) - err = yaml.Unmarshal([]byte(operator.HelmValues), operatorHelmValues) - if err != nil { - return nil, fmt.Errorf("Unmarshalling Helm values failed for operator %s: %w.", operator.GitRef, err) - } - - chartutil.CoalesceTables(operatorValues, operatorHelmValues) - helmValues = append(helmValues, operatorValues) - } - return helmValues, nil -} - // ACSOperatorManager keeps data necessary for managing ACS Operator type ACSOperatorManager struct { client ctrlClient.Client @@ -77,7 +35,6 @@ func (u *ACSOperatorManager) InstallOrUpgrade(ctx context.Context, operators Ope if err != nil { return err } - for _, obj := range objs { if obj.GetNamespace() == "" { obj.SetNamespace(ACSOperatorNamespace) @@ -87,9 +44,7 @@ func (u *ACSOperatorManager) InstallOrUpgrade(ctx context.Context, operators Ope return fmt.Errorf("failed to update operator object %w", err) } } - return nil - } // RenderChart renders the operator helm chart manifests @@ -97,26 +52,16 @@ func RenderChart(operators OperatorConfigs) ([]*unstructured.Unstructured, error if len(operators.Configs) == 0 { return nil, nil } - - operatorImages, err := parseOperatorConfigs(operators) - if err != nil { - return nil, fmt.Errorf("failed to parse images: %w", err) + var valuesArr []chartutil.Values + for _, operator := range operators.Configs { + valuesArr = append(valuesArr, chartutil.Values(operator)) } chartVals := chartutil.Values{ "operator": chartutil.Values{ - "images": operatorImages, + "images": valuesArr, }, } - - var dynamicTemplatesUrls []string - if operators.CRD.GitRef != "" { - dynamicTemplatesUrls, err = generateCRDTemplateUrls(operators.CRD) - if err != nil { - return nil, err - } - } - - resourcesChart, err := charts.GetChart("rhacs-operator", dynamicTemplatesUrls) + resourcesChart, err := charts.GetChart("rhacs-operator", operators.CRDURLs) if err != nil { return nil, fmt.Errorf("failed getting chart: %w", err) } @@ -207,37 +152,6 @@ func (u *ACSOperatorManager) ReadOperatorConfigFromConfigMap(ctx context.Context return configMapOperators, nil } -func generateDeploymentName(version string) string { - return operatorDeploymentPrefix + "-" + version -} - -func generateCRDTemplateUrls(crdConfig CRDConfig) ([]string, error) { - baseURL := defaultCRDBaseURLTemplate - if crdConfig.BaseURL != "" { - baseURL = crdConfig.BaseURL - } - - wr := new(strings.Builder) - crdURLTpl, err := template.New("crd_base_url").Parse(baseURL) - if err != nil { - return []string{}, fmt.Errorf("could not parse CRD base URL: %w", err) - } - - err = crdURLTpl.Execute(wr, struct { - GitRef string - }{ - GitRef: crdConfig.GitRef, - }) - if err != nil { - return []string{}, fmt.Errorf("could not parse CRD base URL: %w", err) - } - - //stackroxWithTag := fmt.Sprintf(baseURL, crdConfig.GitRef) - centralCrdURL := wr.String() + "platform.stackrox.io_centrals.yaml" - securedClusterCrdURL := wr.String() + "platform.stackrox.io_securedclusters.yaml" - return []string{centralCrdURL, securedClusterCrdURL}, nil -} - // NewACSOperatorManager creates a new ACS Operator Manager func NewACSOperatorManager(k8sClient ctrlClient.Client) *ACSOperatorManager { return &ACSOperatorManager{ diff --git a/fleetshard/pkg/central/operator/upgrade_test.go b/fleetshard/pkg/central/operator/upgrade_test.go index 37b9f1b056..c03b6701bd 100644 --- a/fleetshard/pkg/central/operator/upgrade_test.go +++ b/fleetshard/pkg/central/operator/upgrade_test.go @@ -2,10 +2,10 @@ package operator import ( "context" + "fmt" + "strings" "testing" - "helm.sh/helm/v3/pkg/chartutil" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -20,31 +20,24 @@ import ( ) const ( - kindCRDName = "CustomResourceDefinition" - k8sAPIVersion = "apiextensions.k8s.io/v1" - operatorRepository = "quay.io/rhacs-eng/stackrox-operator" - operatorImage1 = "quay.io/rhacs-eng/stackrox-operator:4.0.1" - operatorImage2 = "quay.io/rhacs-eng/stackrox-operator:4.0.2" - crdURL = "https://raw.githubusercontent.com/stackrox/stackrox/%s/operator/bundle/manifests/" - deploymentName1 = operatorDeploymentPrefix + "-4.0.1" - deploymentName2 = operatorDeploymentPrefix + "-4.0.2" + kindCRDName = "CustomResourceDefinition" + k8sAPIVersion = "apiextensions.k8s.io/v1" + operatorImage1 = "quay.io/rhacs-eng/stackrox-operator:4.0.1" + operatorImage2 = "quay.io/rhacs-eng/stackrox-operator:4.0.2" + deploymentName1 = operatorDeploymentPrefix + "-4-0-1" + deploymentName2 = operatorDeploymentPrefix + "-4-0-2" ) -var crdTag1 = OperatorConfigs{ - CRD: CRDConfig{ - GitRef: "4.0.1", - }, +func operatorConfigForVersion(version string) OperatorConfig { + return OperatorConfig{ + "image": fmt.Sprintf("quay.io/rhacs-eng/stackrox-operator:%s", version), + "deploymentName": "rhacs-operator-" + strings.ReplaceAll(version, ".", "-"), + } } -var operatorConfig1 = OperatorConfig{ - Image: operatorImage1, - GitRef: "4.0.1", -} +var operatorConfig1 = operatorConfigForVersion("4.0.1") -var operatorConfig2 = OperatorConfig{ - Image: operatorImage2, - GitRef: "4.0.2", -} +var operatorConfig2 = operatorConfigForVersion("4.0.2") var securedClusterCRD = &unstructured.Unstructured{ Object: map[string]interface{}{ @@ -111,8 +104,9 @@ func createOperatorDeployment(name string, image string) *appsv1.Deployment { func getExampleOperatorConfigs(configs ...OperatorConfig) OperatorConfigs { return OperatorConfigs{ - CRD: CRDConfig{ - GitRef: "4.0.1", + CRDURLs: []string{ + "https://raw.githubusercontent.com/stackrox/stackrox/4.1.2/operator/bundle/manifests/platform.stackrox.io_securedclusters.yaml", + "https://raw.githubusercontent.com/stackrox/stackrox/4.1.2/operator/bundle/manifests/platform.stackrox.io_centrals.yaml", }, Configs: configs, } @@ -180,34 +174,13 @@ func TestOperatorUpgradeMultipleVersions(t *testing.T) { assert.Equal(t, managerContainer.Image, operatorImage2) } -func TestOperatorUpgradeDoNotInstallLongTagVersion(t *testing.T) { - fakeClient := testutils.NewFakeClientBuilder(t).Build() - u := NewACSOperatorManager(fakeClient) - - longVersionName := "4.0.1-with-ridiculously-long-version-name-like-really-long-one-which-has-more-than-63-characters" - operatorConfig := OperatorConfig{ - Image: operatorImage1, - GitRef: longVersionName, - } - - err := u.InstallOrUpgrade(context.Background(), getExampleOperatorConfigs(operatorConfig)) - require.Errorf(t, err, "zero tags parsed from images") - - deployments := &appsv1.DeploymentList{} - err = fakeClient.List(context.Background(), deployments) - require.NoError(t, err) - assert.Len(t, deployments.Items, 0) -} - func TestOperatorUpgradeImageWithDigest(t *testing.T) { fakeClient := testutils.NewFakeClientBuilder(t).Build() u := NewACSOperatorManager(fakeClient) digestedImage := "quay.io/rhacs-eng/stackrox-operator:4.0.1@sha256:232a180dbcbcfa7250917507f3827d88a9ae89bb1cdd8fe3ac4db7b764ebb25a" - operatorConfig := OperatorConfig{ - Image: digestedImage, - GitRef: "4.0.1", - } + operatorConfig := operatorConfigForVersion("4.0.1") + operatorConfig["image"] = digestedImage err := u.InstallOrUpgrade(context.Background(), getExampleOperatorConfigs(operatorConfig)) require.NoError(t, err) @@ -285,96 +258,3 @@ func TestRemoveMultipleUnusedOperators(t *testing.T) { err = fakeClient.Get(context.Background(), client.ObjectKey{Namespace: ACSOperatorNamespace, Name: serviceAccount.GetName()}, serviceAccount) require.NoError(t, err) } - -func TestParseOperatorConfigs(t *testing.T) { - cases := map[string]struct { - operatorConfigs OperatorConfigs - expected []map[string]string - shouldFail bool - }{ - "should parse one valid operator image": { - operatorConfigs: getExampleOperatorConfigs(operatorConfig1), - expected: []map[string]string{ - {"deploymentName": deploymentName1, "image": operatorImage1, "labelSelector": "4.0.1"}, - }, - }, - "should parse two valid operator images": { - operatorConfigs: getExampleOperatorConfigs(operatorConfig1, operatorConfig2), - expected: []map[string]string{ - {"deploymentName": deploymentName1, "image": operatorImage1, "labelSelector": "4.0.1"}, - {"deploymentName": deploymentName2, "image": operatorImage2, "labelSelector": "4.0.2"}, - }, - }, - "should parse image with tag and digest": { - operatorConfigs: getExampleOperatorConfigs(OperatorConfig{ - Image: "quay.io/image-with-tag-and-digest:1.2.3@sha256:4ff5cb2dcddaaa2a4b702516870c85177e53ccc3566509c36c2d84b01ef8f783", - GitRef: "version1", - }), - expected: []map[string]string{ - { - "deploymentName": operatorDeploymentPrefix + "-version1", - "image": "quay.io/image-with-tag-and-digest:1.2.3@sha256:4ff5cb2dcddaaa2a4b702516870c85177e53ccc3566509c36c2d84b01ef8f783", - "labelSelector": "version1", - }, - }, - }, - "should parse image without tag": { - operatorConfigs: getExampleOperatorConfigs(OperatorConfig{ - Image: "quay.io/image-without-tag", - GitRef: "version1", - }), - expected: []map[string]string{ - {"deploymentName": operatorDeploymentPrefix + "-version1", "image": "quay.io/image-without-tag", "labelSelector": "version1"}, - }, - }, - "do not fail if images list is empty": { - operatorConfigs: getExampleOperatorConfigs(), - shouldFail: false, - }, - "should accept images from multiple repositories with the same tag": { - operatorConfigs: getExampleOperatorConfigs( - OperatorConfig{Image: "repo1:tag", GitRef: "version1"}, - OperatorConfig{Image: "repo2:tag", GitRef: "version2"}, - ), - expected: []map[string]string{ - {"deploymentName": operatorDeploymentPrefix + "-version1", "image": "repo1:tag", "labelSelector": "version1"}, - {"deploymentName": operatorDeploymentPrefix + "-version2", "image": "repo2:tag", "labelSelector": "version2"}, - }, - }, - "fail if image contains more than one colon": { - operatorConfigs: getExampleOperatorConfigs(OperatorConfig{ - Image: "quay.io/image-name:1.2.3:", - GitRef: "version1", - }), - shouldFail: true, - }, - "fail if GitRef is way too long for the DeploymentName": { - operatorConfigs: getExampleOperatorConfigs(OperatorConfig{ - Image: "quay.io/image-name:tag", - GitRef: "1.2.3-with-ridiculously-long-version-name-like-really-long-one-which-has-more-than-63-characters", - }), - shouldFail: true, - }, - } - - for name, c := range cases { - t.Run(name, func(t *testing.T) { - gotImages, err := parseOperatorConfigs(c.operatorConfigs) - if c.shouldFail { - assert.Error(t, err) - } else { - assert.NoError(t, err) - var expectedRepositoryAndTags []chartutil.Values - for _, m := range c.expected { - val := chartutil.Values{ - "deploymentName": m["deploymentName"], - "image": m["image"], - "labelSelector": m["labelSelector"], - } - expectedRepositoryAndTags = append(expectedRepositoryAndTags, val) - } - assert.Equal(t, expectedRepositoryAndTags, gotImages) - } - }) - } -} diff --git a/fleetshard/pkg/central/operator/validation.go b/fleetshard/pkg/central/operator/validation.go new file mode 100644 index 0000000000..4876c834a9 --- /dev/null +++ b/fleetshard/pkg/central/operator/validation.go @@ -0,0 +1,171 @@ +package operator + +import ( + "fmt" + "net/url" + + "github.com/containers/image/docker/reference" + "k8s.io/apimachinery/pkg/api/validation" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +// Validate validates the operator configuration and can be used in different life-cycle stages like runtime and deploy time. +func Validate(path *field.Path, configs OperatorConfigs) field.ErrorList { + var errs field.ErrorList + errs = append(errs, validateCRDUrls(path.Child("crdUrls"), configs.CRDURLs)...) + errs = append(errs, validateOperatorConfigs(path.Child("operators"), configs.Configs)...) + if len(errs) == 0 { + errs = append(errs, validateManifests(path, configs)...) + } + return errs +} + +func validateManifests(path *field.Path, configs OperatorConfigs) field.ErrorList { + var errs field.ErrorList + manifests, err := RenderChart(configs) + if err != nil { + errs = append(errs, field.Invalid(path, nil, fmt.Sprintf("could not render operator helm charts: %s", err.Error()))) + return errs + } + if len(configs.Configs) > 0 && len(manifests) == 0 { + errs = append(errs, field.Invalid(path, nil, "operator chart rendering succeed, but no manifests were rendered")) + return errs + } + return nil +} + +func validateCRDUrls(path *field.Path, urls []string) field.ErrorList { + var errs field.ErrorList + for i, urlStr := range urls { + errs = append(errs, validateCRDURL(path.Index(i), urlStr)...) + } + return errs +} + +func validateCRDURL(path *field.Path, urlStr string) field.ErrorList { + var errs field.ErrorList + if _, err := url.Parse(urlStr); err != nil { + errs = append(errs, field.Invalid(path, urlStr, fmt.Sprintf("invalid url: %s", err.Error()))) + } + return errs +} + +func validateOperatorConfigs(path *field.Path, configs []OperatorConfig) field.ErrorList { + var errs field.ErrorList + seenDeploymentNames := make(map[string]struct{}) + for i, config := range configs { + // Check that each deployment name is unique + if _, ok := seenDeploymentNames[config.GetDeploymentName()]; ok { + errs = append(errs, field.Duplicate(path.Index(i).Child("deploymentName"), config.GetDeploymentName())) + } + seenDeploymentNames[config.GetDeploymentName()] = struct{}{} + errs = append(errs, validateOperatorConfig(path.Index(i), config)...) + } + return errs +} + +func validateOperatorConfig(path *field.Path, config OperatorConfig) field.ErrorList { + var errs field.ErrorList + errs = append(errs, validateDeploymentName(path.Child(keyDeploymentName), config[keyDeploymentName])...) + errs = append(errs, validateImage(path.Child(keyImage), config[keyImage])...) + errs = append(errs, validateCentralReconcilerEnabled(path.Child(keyCentralReconcilerEnabled), config[keyCentralReconcilerEnabled])...) + errs = append(errs, validateSecuredClusterReconcilerEnabled(path.Child(keySecuredClusterReconcilerEnabled), config[keySecuredClusterReconcilerEnabled])...) + errs = append(errs, validateLabelSelector(path.Child(keyCentralLabelSelector), config[keyCentralLabelSelector])...) + errs = append(errs, validateLabelSelector(path.Child(keySecuredClusterSelector), config[keySecuredClusterSelector])...) + errs = append(errs, validateHasCentralSelectorOrReconcilerDisabled(path, config)...) + errs = append(errs, validateHasSecuredClusterSelectorOrReconcilerDisabled(path, config)...) + return errs +} + +func validateHasCentralSelectorOrReconcilerDisabled(path *field.Path, config OperatorConfig) field.ErrorList { + var errs field.ErrorList + if len(config.GetCentralLabelSelector()) == 0 && config.GetCentralReconcilerEnabled() { + errs = append(errs, field.Invalid(path, nil, "central label selector must be specified or central reconciler must be disabled")) + } + return errs +} + +func validateHasSecuredClusterSelectorOrReconcilerDisabled(path *field.Path, config OperatorConfig) field.ErrorList { + var errs field.ErrorList + if len(config.GetSecuredClusterLabelSelector()) == 0 && config.GetSecuredClusterReconcilerEnabled() { + errs = append(errs, field.Invalid(path, nil, "secured cluster label selector must be specified or secured cluster reconciler must be disabled")) + } + return errs +} + +func validateDeploymentName(path *field.Path, deploymentNameIntf interface{}) field.ErrorList { + var errs field.ErrorList + deploymentName, ok := deploymentNameIntf.(string) + if !ok { + errs = append(errs, field.Invalid(path, deploymentName, "deployment name must be a string")) + } + if len(deploymentName) == 0 { + errs = append(errs, field.Invalid(path, deploymentName, "deployment name cannot be empty")) + return errs + } + if dnsErrs := validation.NameIsDNSSubdomain(deploymentName, true); len(dnsErrs) > 0 { + errs = append(errs, field.Invalid(path, deploymentName, fmt.Sprintf("invalid deployment name: %v", dnsErrs[0]))) + } + return errs +} + +func validateImage(path *field.Path, imageIntf interface{}) field.ErrorList { + var errs field.ErrorList + image, ok := imageIntf.(string) + if !ok { + errs = append(errs, field.Invalid(path, image, "image must be a string")) + return errs + } + if len(image) == 0 { + errs = append(errs, field.Invalid(path, image, "image cannot be empty")) + return errs + } + _, err := reference.Parse(image) + if err != nil { + errs = append(errs, field.Invalid(path, image, fmt.Sprintf("invalid image: %v", err))) + return errs + } + return errs +} + +func validateCentralReconcilerEnabled(path *field.Path, centralReconcilerEnabledIntf interface{}) field.ErrorList { + var errs field.ErrorList + if centralReconcilerEnabledIntf == nil { + return nil + } + centralReconcilerEnabled, ok := centralReconcilerEnabledIntf.(bool) + if !ok { + errs = append(errs, field.Invalid(path, centralReconcilerEnabled, "centralReconcilerEnabled must be a boolean")) + } + return errs +} + +func validateSecuredClusterReconcilerEnabled(path *field.Path, securedClusterReconcilerEnabledIntf interface{}) field.ErrorList { + var errs field.ErrorList + if securedClusterReconcilerEnabledIntf == nil { + return nil + } + securedClusterReconcilerEnabled, ok := securedClusterReconcilerEnabledIntf.(bool) + if !ok { + errs = append(errs, field.Invalid(path, securedClusterReconcilerEnabled, "securedClusterReconcilerEnabled must be a boolean")) + } + return errs +} + +func validateLabelSelector(path *field.Path, selectorIntf interface{}) field.ErrorList { + var errs field.ErrorList + if selectorIntf == nil { + return nil + } + selectorStr, ok := selectorIntf.(string) + if !ok { + errs = append(errs, field.Invalid(path, selectorStr, "label selector must be a string")) + return errs + } + _, err := v1.ParseToLabelSelector(selectorStr) + if err != nil { + errs = append(errs, field.Invalid(path, selectorStr, fmt.Sprintf("invalid label selector: %v", err))) + } + return errs +} diff --git a/fleetshard/pkg/central/operator/validation_test.go b/fleetshard/pkg/central/operator/validation_test.go new file mode 100644 index 0000000000..e9c92afece --- /dev/null +++ b/fleetshard/pkg/central/operator/validation_test.go @@ -0,0 +1,159 @@ +package operator + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/util/validation/field" +) + +func TestGetOperatorConfigFailsValidation(t *testing.T) { + testCases := map[string]struct { + getConfig func(*testing.T, OperatorConfigs) OperatorConfigs + contains string + success bool + }{ + "should fail with invalid crd url": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + config.CRDURLs = []string{ + "invalid url", + } + return config + }, + contains: "failed downloading chart files", + }, + "should fail with empty deployment name": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg := validOperatorConfig() + cfg[keyDeploymentName] = "" + config.Configs = []OperatorConfig{cfg} + return config + }, + contains: "deployment name cannot be empty", + }, + "should fail with invalid deployment name": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg := validOperatorConfig() + cfg[keyDeploymentName] = "!!" + config.Configs = []OperatorConfig{cfg} + return config + }, + contains: "invalid deployment name", + }, + "should fail with empty image": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg := validOperatorConfig() + cfg[keyImage] = "" + config.Configs = []OperatorConfig{cfg} + return config + }, + contains: "image cannot be empty", + }, + "should fail with invalid image": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg := validOperatorConfig() + cfg[keyImage] = "??" + config.Configs = []OperatorConfig{cfg} + return config + }, + contains: "invalid image", + }, + "should fail with duplicate deployment names image": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg1 := validOperatorConfig() + cfg1[keyDeploymentName] = "duplicate" + cfg2 := validOperatorConfig() + cfg2[keyDeploymentName] = "duplicate" + config.Configs = []OperatorConfig{cfg1, cfg2} + return config + }, + contains: "rhacsOperator.operators[1].deploymentName: Duplicate value", + }, + "should fail if central selector is empty but reconciler is not disabled": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg := validOperatorConfig() + cfg[keyCentralLabelSelector] = "" + cfg[keyCentralReconcilerEnabled] = true + config.Configs = []OperatorConfig{cfg} + return config + }, + contains: "central label selector must be specified or central reconciler must be disabled", + }, + "should fail if secured cluster selector is empty but reconciler is not disabled": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg := validOperatorConfig() + cfg[keySecuredClusterSelector] = "" + cfg[keySecuredClusterReconcilerEnabled] = true + config.Configs = []OperatorConfig{cfg} + return config + }, + contains: "secured cluster label selector must be specified or secured cluster reconciler must be disabled", + }, + "valid if central label selector is not specified and reconciler is disabled": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg := validOperatorConfig() + cfg[keyCentralLabelSelector] = "" + cfg[keyCentralReconcilerEnabled] = false + config.Configs = []OperatorConfig{cfg} + return config + }, + }, + "valid if secured cluster label selector is not specified and reconciler is disabled": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg := validOperatorConfig() + cfg[keySecuredClusterSelector] = "" + cfg[keySecuredClusterReconcilerEnabled] = false + config.Configs = []OperatorConfig{cfg} + return config + }, + }, + "valid if central label selector is specified and reconciler is not disabled": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg := validOperatorConfig() + cfg[keyCentralLabelSelector] = "app.kubernetes.io/name=central" + cfg[keyCentralReconcilerEnabled] = false + config.Configs = []OperatorConfig{cfg} + return config + }, + }, + "valid if secured cluster label selector is specified and reconciler is not disabled": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + cfg := validOperatorConfig() + cfg[keySecuredClusterSelector] = "app.kubernetes.io/name=securedCluster" + cfg[keySecuredClusterReconcilerEnabled] = false + config.Configs = []OperatorConfig{cfg} + return config + }, + }, + "validate should succeed with example config": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + return config + }, + success: true, + }, + "should succeed with empty operator configs": { + getConfig: func(t *testing.T, config OperatorConfigs) OperatorConfigs { + config.Configs = []OperatorConfig{} + return config + }, + success: true, + }, + } + + for key, testCase := range testCases { + t.Run(key, func(t *testing.T) { + config, err := parseConfig(getExampleConfig()) + require.NoError(t, err) + + errList := Validate(field.NewPath("rhacsOperator"), testCase.getConfig(t, config)) + if testCase.contains != "" { + require.Len(t, errList, 1) + require.NotEmpty(t, testCase.contains) + assert.Contains(t, errList.ToAggregate().Errors()[0].Error(), testCase.contains) + } else { + require.Nil(t, errList, "unexpected error: %v", errList.ToAggregate()) + } + }) + } +} diff --git a/fleetshard/pkg/central/reconciler/reconciler.go b/fleetshard/pkg/central/reconciler/reconciler.go index 5e56b704ad..4221e81662 100644 --- a/fleetshard/pkg/central/reconciler/reconciler.go +++ b/fleetshard/pkg/central/reconciler/reconciler.go @@ -52,8 +52,7 @@ const ( FreeStatus int32 = iota BlockedStatus - PauseReconcileAnnotation = "stackrox.io/pause-reconcile" - ReconcileOperatorSelector = "rhacs.redhat.com/version-selector" + PauseReconcileAnnotation = "stackrox.io/pause-reconcile" helmReleaseName = "tenant-resources" diff --git a/fleetshard/pkg/runtime/runtime.go b/fleetshard/pkg/runtime/runtime.go index ace27ad11f..1fca55aa72 100644 --- a/fleetshard/pkg/runtime/runtime.go +++ b/fleetshard/pkg/runtime/runtime.go @@ -267,27 +267,14 @@ func (r *Runtime) upgradeOperator(list private.ManagedCentralList) error { } glog.Infof("Reading operator config map, extracted %d operators from configmap", len(configMapOperators)) desiredOperatorConfigs = configMapOperators - } else if features.TargetedOperatorUpgrades.Enabled() { + } else { for _, operatorConfig := range list.RhacsOperators.RHACSOperatorConfigs { - desiredOperatorConfigs = append(desiredOperatorConfigs, operator.OperatorConfig{ - Image: operatorConfig.Image, - GitRef: operatorConfig.GitRef, - HelmValues: operatorConfig.HelmValues, - }) + desiredOperatorConfigs = append(desiredOperatorConfigs, operatorConfig) } - } else { - desiredOperatorConfigs = []operator.OperatorConfig{{ - GitRef: "4.1.0", - Image: "quay.io/rhacs-eng/stackrox-operator", - }} } operators := operator.OperatorConfigs{ Configs: desiredOperatorConfigs, - // TODO: How to get that value? - CRD: operator.CRDConfig{ - GitRef: "4.1.0", - }, } if reflect.DeepEqual(cachedOperatorConfigs, operators) { @@ -296,8 +283,8 @@ func (r *Runtime) upgradeOperator(list private.ManagedCentralList) error { cachedOperatorConfigs = operators for _, operatorDeployment := range operators.Configs { - glog.Infof("Installing Operator version: %s", operatorDeployment.GitRef) - desiredOperatorImages = append(desiredOperatorImages, operatorDeployment.Image) + glog.Infof("Installing Operator: %s", operatorDeployment.GetImage()) + desiredOperatorImages = append(desiredOperatorImages, operatorDeployment.GetImage()) } // TODO: comment line in to use the API response for production usage after Fleet-Manager implementation is finished @@ -307,6 +294,7 @@ func (r *Runtime) upgradeOperator(list private.ManagedCentralList) error { return fmt.Errorf("ensuring initial operator installation failed: %w", err) } + // TODO: Is this needed? Wouldn't helm take care of cleanup? err = r.operatorManager.RemoveUnusedOperators(ctx, desiredOperatorImages) if err != nil { glog.Warningf("Failed removing unused operators: %v", err) diff --git a/internal/dinosaur/pkg/api/private/api/openapi.yaml b/internal/dinosaur/pkg/api/private/api/openapi.yaml index f3a448de89..f3a88f9334 100644 --- a/internal/dinosaur/pkg/api/private/api/openapi.yaml +++ b/internal/dinosaur/pkg/api/private/api/openapi.yaml @@ -349,22 +349,14 @@ components: description: A list of ManagedCentral RHACSOperatorConfigs: properties: - CRD: - $ref: '#/components/schemas/RHACSOperatorConfigs_CRD' + CrdUrls: + items: + type: string + type: array RHACSOperatorConfigs: items: - allOf: - - $ref: '#/components/schemas/RHACSOperatorConfig' + type: object type: array - RHACSOperatorConfig: - description: RHACSOperatorConfig defines the configuration of an operator - properties: - gitRef: - type: string - image: - type: string - helmValues: - type: string DataPlaneClusterUpdateStatusRequest: description: Schema for the request to update a data plane cluster's status example: @@ -583,12 +575,6 @@ components: type: array rhacs_operators: $ref: '#/components/schemas/RHACSOperatorConfigs' - RHACSOperatorConfigs_CRD: - properties: - baseURL: - type: string - gitRef: - type: string DataPlaneClusterUpdateStatusRequest_conditions: example: reason: reason diff --git a/internal/dinosaur/pkg/api/private/model_rhacs_operator_config.go b/internal/dinosaur/pkg/api/private/model_rhacs_operator_config.go deleted file mode 100644 index 00dc8e24c8..0000000000 --- a/internal/dinosaur/pkg/api/private/model_rhacs_operator_config.go +++ /dev/null @@ -1,18 +0,0 @@ -/* - * Red Hat Advanced Cluster Security Service Fleet Manager - * - * Red Hat Advanced Cluster Security (RHACS) Service Fleet Manager APIs that are used by internal services e.g fleetshard operators. - * - * API version: 1.4.0 - * Generated by: OpenAPI Generator (https://openapi-generator.tech) - */ - -// Code generated by OpenAPI Generator (https://openapi-generator.tech). DO NOT EDIT. -package private - -// RhacsOperatorConfig RHACSOperatorConfig defines the configuration of an operator -type RhacsOperatorConfig struct { - GitRef string `json:"gitRef,omitempty"` - Image string `json:"image,omitempty"` - HelmValues string `json:"helmValues,omitempty"` -} diff --git a/internal/dinosaur/pkg/api/private/model_rhacs_operator_configs.go b/internal/dinosaur/pkg/api/private/model_rhacs_operator_configs.go index 66adaed154..c25c93d48f 100644 --- a/internal/dinosaur/pkg/api/private/model_rhacs_operator_configs.go +++ b/internal/dinosaur/pkg/api/private/model_rhacs_operator_configs.go @@ -12,6 +12,6 @@ package private // RhacsOperatorConfigs struct for RhacsOperatorConfigs type RhacsOperatorConfigs struct { - CRD RhacsOperatorConfigsCrd `json:"CRD,omitempty"` - RHACSOperatorConfigs []RhacsOperatorConfig `json:"RHACSOperatorConfigs,omitempty"` + CrdUrls []string `json:"CrdUrls,omitempty"` + RHACSOperatorConfigs []map[string]interface{} `json:"RHACSOperatorConfigs,omitempty"` } diff --git a/internal/dinosaur/pkg/api/private/model_rhacs_operator_configs_crd.go b/internal/dinosaur/pkg/api/private/model_rhacs_operator_configs_crd.go deleted file mode 100644 index f0740735c8..0000000000 --- a/internal/dinosaur/pkg/api/private/model_rhacs_operator_configs_crd.go +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Red Hat Advanced Cluster Security Service Fleet Manager - * - * Red Hat Advanced Cluster Security (RHACS) Service Fleet Manager APIs that are used by internal services e.g fleetshard operators. - * - * API version: 1.4.0 - * Generated by: OpenAPI Generator (https://openapi-generator.tech) - */ - -// Code generated by OpenAPI Generator (https://openapi-generator.tech). DO NOT EDIT. -package private - -// RhacsOperatorConfigsCrd struct for RhacsOperatorConfigsCrd -type RhacsOperatorConfigsCrd struct { - BaseURL string `json:"baseURL,omitempty"` - GitRef string `json:"gitRef,omitempty"` -} diff --git a/internal/dinosaur/pkg/gitops/config_test.go b/internal/dinosaur/pkg/gitops/config_test.go index c311ce2dac..1120aa2f45 100644 --- a/internal/dinosaur/pkg/gitops/config_test.go +++ b/internal/dinosaur/pkg/gitops/config_test.go @@ -24,12 +24,13 @@ func TestValidateGitOpsConfig(t *testing.T) { }, yaml: ` rhacsOperators: - crd: - baseURL: https://raw.githubusercontent.com/stackrox/stackrox/{{ .GitRef }}/operator/bundle/manifests/ - gitRef: 4.1.1 + crdURls: + - https://raw.githubusercontent.com/stackrox/stackrox/4.1.2/operator/bundle/manifests/platform.stackrox.io_securedclusters.yaml operators: - - gitRef: 4.1.1 - image: "quay.io/rhacs-eng/stackrox-operator:4.1.1" + - image: "quay.io/rhacs-eng/stackrox-operator:4.1.1" + deploymentName: "stackrox-operator" + centralLabelSelector: "app.kubernetes.io/name=central" + securedClusterLabelSelector: "app.kubernetes.io/name=securedCluster" centrals: overrides: - instanceIds: @@ -43,13 +44,6 @@ centrals: assert.Equal(t, field.Invalid(field.NewPath("centrals", "overrides").Index(0).Child("patch"), "foo", "invalid patch: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go value of type v1alpha1.Central"), err[0]) }, yaml: ` -rhacsOperators: - crd: - baseURL: https://raw.githubusercontent.com/stackrox/stackrox/{{ .GitRef }}/operator/bundle/manifests/ - gitRef: 4.1.1 - operators: - - gitRef: 4.1.1 - image: "quay.io/rhacs-eng/stackrox-operator:4.1.1" centrals: overrides: - instanceIds: @@ -63,13 +57,6 @@ centrals: assert.Equal(t, field.Invalid(field.NewPath("centrals", "overrides").Index(0).Child("patch"), "spec: 123\n", "invalid patch: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal number into Go struct field Central.spec of type v1alpha1.CentralSpec"), err[0]) }, yaml: ` -rhacsOperators: - crd: - baseURL: https://raw.githubusercontent.com/stackrox/stackrox/{{ .GitRef }}/operator/bundle/manifests/ - gitRef: 4.1.1 - operators: - - gitRef: 4.1.1 - image: "quay.io/rhacs-eng/stackrox-operator:4.1.1" centrals: overrides: - instanceIds: @@ -83,9 +70,6 @@ centrals: assert.Contains(t, err.ToAggregate().Errors()[0].Error(), "cannot unmarshal string into Go value of type v1alpha1.Central", "central config was not validated") }, yaml: ` -rhacsOperators: - crd: - baseURL: invalid centrals: overrides: - instanceIds: diff --git a/openapi/fleet-manager-private.yaml b/openapi/fleet-manager-private.yaml index 56a2f52e0e..26b2ccaa2f 100644 --- a/openapi/fleet-manager-private.yaml +++ b/openapi/fleet-manager-private.yaml @@ -395,29 +395,15 @@ components: RHACSOperatorConfigs: properties: - CRD: - type: object - properties: - baseURL: - type: string - gitRef: + CrdUrls: + type: array + items: type: string RHACSOperatorConfigs: type: array items: - allOf: - - $ref: '#/components/schemas/RHACSOperatorConfig' + type: object - RHACSOperatorConfig: - description: >- - RHACSOperatorConfig defines the configuration of an operator - properties: - gitRef: - type: string - image: - type: string - helmValues: - type: string DataPlaneClusterUpdateStatusRequest: # TODO are there any fields that should be required? # TODO are there any fields that should be nullable? (this is, a pointer in the resulting generated Go code)