ROX-18942 - Add operator configuration in fleetshard-sync #1157

SimonBaeumer · 2023-07-17T09:25:25Z

Description

This PR adds a declarative structure to configure RHACS operators to deploy to a data-plane cluster.
OperatorConfigs represents all Operators including a CRD which should be installed.
The Operators are a slice of OperatorConfig which is applied to the Helm chart.
The Validate function executes extended validation when the configuration is read to provide early failures and being used from a CLI context.
It also validates the Helm chart rendering including CRD download.

The API call to the private Central list call, polled by fleetshard, is extended with a property for the operator configurations.

Data Structure

Example structure:

crd:
  baseURL: https://raw.githubusercontent.com/stackrox/stackrox/%s/operator/bundle/manifests/
  gitRef: 4.1.1
operators:
- gitRef: 4.1.1
  image: "quay.io/rhacs-eng/stackrox-operator:4.1.1"
  helmValues: |
    operator:
      resources:
        requests:
          memory: 500Mi
          cpu: 50m

Checklist (Definition of Done)

Unit and integration tests added
Added test description under Test manual
Documentation added if necessary (i.e. changes to dev setup, test execution, ...)
CI and all relevant tests are passing
Add the ticket number to the PR title if available, i.e. ROX-12345: ...
Discussed security and business related topics privately. Will move any security and business related topics that arise to private communication channel.
Add secret to app-interface Vault or Secrets Manager if necessary
RDS changes were e2e tested manually
Check AWS limits are reasonable for changes provisioning new resources

TODO

Test provisioning to a data-plane cluster
Add more test cases and validations
- Test helm chart values are passed to Helm chart
- More validation tests

Test manual

Added unit tests

openshift-ci · 2023-07-17T09:25:29Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

SimonBaeumer · 2023-07-17T11:10:25Z

dev/env/manifests/fleetshard-sync/02-fleetshard-sync-config.yaml

+    rhacs_operator:
+      resources:
+        limits:
+          cpu: 0.5
+          memory: "1GiB"
+        requests:
+          cpu: 50m
+          memory: "250MiB"


Suggested change

rhacs_operator:

resources:

limits:

cpu: 0.5

memory: "1GiB"

requests:

cpu: 50m

memory: "250MiB"

rhacs_operator:

default:

resources:

limits:

cpu: 0.5

memory: "1GiB"

requests:

cpu: 50m

memory: "250MiB"

quay.io/rhacs-eng/stackrox@sha256:asdfasdfsadfasdfasf:

resources:

limits:

cpu: 0.5

memory: "1GiB"

requests:

cpu: 50m

memory: "250MiB"

ludydoo · 2023-07-20T09:23:43Z

fleetshard/pkg/central/charts/data/rhacs-operator/templates/rhacs-operator-deployment.yaml

@@ -115,11 +115,11 @@ spec:
            timeoutSeconds: 1
          resources:
            limits:
-              cpu: 2


Can you put those under operator.resources...? Because we might want to also configure the rbac-proxy (e.g. rbac-proxy.resources...)

Yes, good point

ludydoo · 2023-07-20T09:25:25Z

dev/env/manifests/fleetshard-sync/02-fleetshard-sync-config.yaml

@@ -0,0 +1,15 @@
+---


Will this be the git-ops config map as well ?

Nope, this will be removed

ludydoo · 2023-07-20T09:36:16Z

dev/env/manifests/fleetshard-sync/02-fleetshard-sync-config.yaml

+  name: fleetshard-sync-config
+data:
+  config.yaml: |
+    rhacs_operator:


should this be rather

operators: - image: ... operator: resources: ... rbacProxy: ...

SimonBaeumer · 2023-07-21T08:49:04Z

@ludydoo fyi The PR is out of date with the recent design changes we discussed.

ludydoo · 2023-07-24T09:10:22Z

internal/dinosaur/pkg/services/rhacsoperator/config.go

+	Image      string `yaml:"image"`
+	GitRef     string `yaml:"gitRef"`
+	HelmValues string `yaml:"helmValues,omitempty"`


using json should work too

ludydoo · 2023-07-24T09:10:43Z

internal/dinosaur/pkg/services/rhacsoperator/config.go

+	return []byte(`
+- gitRef: 4.1.1
+  image: "quay.io/rhacs-eng/stackrox-operator:4.0.0"
+  helmValues: |


What's the reason to use a string here and not an object & unmarshaling it into a map[string]interface{} ?

I've not found a way to define an unstructured object to the OpenAPI spec.

fleetshard/pkg/runtime/runtime.go

kurlov

Looking great!

openshift-ci · 2023-08-09T08:24:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kurlov, SimonBaeumer

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [SimonBaeumer,kurlov]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2023-08-09T08:27:27Z

New changes are detected. LGTM label has been removed.

SimonBaeumer · 2023-08-09T09:01:33Z

/retest

ludydoo · 2023-08-09T09:55:27Z

fleetshard/config/config.go

@@ -28,7 +28,7 @@ type Config struct {
 	CreateAuthProvider   bool          `env:"CREATE_AUTH_PROVIDER" envDefault:"false"`
 	MetricsAddress       string        `env:"FLEETSHARD_METRICS_ADDRESS" envDefault:":8080"`
 	EgressProxyImage     string        `env:"EGRESS_PROXY_IMAGE"`
-	BaseCrdURL           string        `env:"BASE_CRD_URL" envDefault:"https://raw.githubusercontent.com/stackrox/stackrox/%s/operator/bundle/manifests/"`
+	DefaultBaseCRDURL    string        `env:"DEFAULT_BASE_CRD_URL" envDefault:"https://raw.githubusercontent.com/stackrox/stackrox/%s/operator/bundle/manifests/"`


Consider something like:

Suggested change

DefaultBaseCRDURL string `env:"DEFAULT_BASE_CRD_URL" envDefault:"https://raw.githubusercontent.com/stackrox/stackrox/%s/operator/bundle/manifests/"`

CentralCRDURLTemplate string `env:"CENTRAL_CRD_URL_TEMPLATE" envDefault:"https://raw.githubusercontent.com/stackrox/stackrox/{{ .GitRef }} /operator/bundle/manifests/platform.stackrox.io_centrals.yaml"`

Removed the variable and replaced it with a constant. We can overwrite the default via GitOps, there is no need for a config as an env variable, it would be duplicated.

ludydoo · 2023-08-09T09:56:49Z

fleetshard/pkg/central/operator/config.go

+	var errors []error
+	manager := ACSOperatorManager{
+		// TODO: align config URL with fleetshard-sync default
+		DefaultBaseCRDURL: "https://raw.githubusercontent.com/stackrox/stackrox/%s/operator/bundle/manifests/",


Why is this duplicated there ?

Removed it and replaced with a constant

openshift-ci bot added the do-not-merge/work-in-progress label Jul 17, 2023

SimonBaeumer temporarily deployed to development July 17, 2023 09:25 — with GitHub Actions Inactive

openshift-ci bot added the approved label Jul 17, 2023

SimonBaeumer commented Jul 17, 2023

View reviewed changes

SimonBaeumer temporarily deployed to development July 20, 2023 08:19 — with GitHub Actions Inactive

ludydoo reviewed Jul 20, 2023

View reviewed changes

SimonBaeumer temporarily deployed to development July 21, 2023 09:01 — with GitHub Actions Inactive

SimonBaeumer temporarily deployed to development July 21, 2023 13:45 — with GitHub Actions Inactive

SimonBaeumer temporarily deployed to development July 24, 2023 09:09 — with GitHub Actions Inactive

ludydoo reviewed Jul 24, 2023

View reviewed changes

SimonBaeumer temporarily deployed to development July 24, 2023 09:38 — with GitHub Actions Inactive

SimonBaeumer marked this pull request as ready for review August 8, 2023 14:18

openshift-ci bot removed the do-not-merge/work-in-progress label Aug 8, 2023

SimonBaeumer temporarily deployed to development August 8, 2023 14:18 — with GitHub Actions Inactive

SimonBaeumer requested a review from kurlov August 8, 2023 14:25

WIP

ebae82c

SimonBaeumer temporarily deployed to development August 9, 2023 08:04 — with GitHub Actions Inactive

SimonBaeumer temporarily deployed to development August 9, 2023 08:05 — with GitHub Actions Inactive

kurlov reviewed Aug 9, 2023

View reviewed changes

fleetshard/pkg/runtime/runtime.go Outdated Show resolved Hide resolved

kurlov approved these changes Aug 9, 2023

View reviewed changes

openshift-ci bot added the lgtm label Aug 9, 2023

WIP

0afbf63

openshift-ci bot removed the lgtm label Aug 9, 2023

SimonBaeumer temporarily deployed to development August 9, 2023 08:27 — with GitHub Actions Inactive

SimonBaeumer changed the title ~~Add operator configuration in fleetshard-sync~~ ROX-18942 - Add operator configuration in fleetshard-sync Aug 9, 2023

ludydoo reviewed Aug 9, 2023

View reviewed changes

WIP

dac51b2

SimonBaeumer temporarily deployed to development August 10, 2023 13:53 — with GitHub Actions Inactive

SimonBaeumer merged commit 63d5d0e into main Aug 11, 2023

SimonBaeumer deleted the sb/operator-configuration branch August 11, 2023 07:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ROX-18942 - Add operator configuration in fleetshard-sync #1157

ROX-18942 - Add operator configuration in fleetshard-sync #1157

SimonBaeumer commented Jul 17, 2023 •

edited

Loading

openshift-ci bot commented Jul 17, 2023

SimonBaeumer Jul 17, 2023

ludydoo Jul 20, 2023

SimonBaeumer Jul 21, 2023

ludydoo Jul 20, 2023

SimonBaeumer Jul 21, 2023

ludydoo Jul 20, 2023

SimonBaeumer Jul 21, 2023

SimonBaeumer commented Jul 21, 2023 •

edited

Loading

ludydoo Jul 24, 2023

ludydoo Jul 24, 2023

SimonBaeumer Aug 8, 2023

kurlov left a comment

openshift-ci bot commented Aug 9, 2023

openshift-ci bot commented Aug 9, 2023

SimonBaeumer commented Aug 9, 2023

ludydoo Aug 9, 2023 •

edited

Loading

SimonBaeumer Aug 11, 2023

ludydoo Aug 9, 2023

SimonBaeumer Aug 11, 2023

	DefaultBaseCRDURL string `env:"DEFAULT_BASE_CRD_URL" envDefault:"https://raw.githubusercontent.com/stackrox/stackrox/%s/operator/bundle/manifests/"`
	CentralCRDURLTemplate string `env:"CENTRAL_CRD_URL_TEMPLATE" envDefault:"https://raw.githubusercontent.com/stackrox/stackrox/{{ .GitRef }} /operator/bundle/manifests/platform.stackrox.io_centrals.yaml"`

ROX-18942 - Add operator configuration in fleetshard-sync #1157

ROX-18942 - Add operator configuration in fleetshard-sync #1157

Conversation

SimonBaeumer commented Jul 17, 2023 • edited Loading

Description

Data Structure

Checklist (Definition of Done)

TODO

Test manual

openshift-ci bot commented Jul 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimonBaeumer commented Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kurlov left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Aug 9, 2023

openshift-ci bot commented Aug 9, 2023

SimonBaeumer commented Aug 9, 2023

ludydoo Aug 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimonBaeumer commented Jul 17, 2023 •

edited

Loading

SimonBaeumer commented Jul 21, 2023 •

edited

Loading

ludydoo Aug 9, 2023 •

edited

Loading