feat: support helm dryrun=server (operator-framework#6691)

holyspectral · web-flow · commit 3969b9b90348 · 2024-03-29T10:38:59.000-04:00
* feat: support helm dryrun=server

Signed-off-by: Sam Wang (holyspectral) &lt;sam.wang@suse.com&gt;

* fix: CI errors after test-sanity

Signed-off-by: Sam Wang (holyspectral) &lt;sam.wang@suse.com&gt;

* docs: update testdata/v3/memcached-operator to v4

Signed-off-by: Sam Wang (holyspectral) &lt;sam.wang@suse.com&gt;

---------

Signed-off-by: Sam Wang (holyspectral) &lt;sam.wang@suse.com&gt;
diff --git a/.cncf-maintainers b/.cncf-maintainers
@@ -1,4 +1,5 @@
 approvers:
+- acornett21
 - anik120
 - everettraven
 - fabianvf
@@ -10,6 +11,7 @@ approvers:
 - theishshah
 - varshaprasad96
 reviewers:
+- acornett21
 - anik120
 - everettraven
 - fabianvf
diff --git a/internal/cmd/helm-operator/run/cmd.go b/internal/cmd/helm-operator/run/cmd.go
@@ -203,6 +203,7 @@ func run(cmd *cobra.Command, f *flags.Flags) {
 			SuppressOverrideValues:  f.SuppressOverrideValues,
 			MaxConcurrentReconciles: f.MaxConcurrentReconciles,
 			Selector:                w.Selector,
+			DryRunOption:            w.DryRunOption,
 		})
 		if err != nil {
 			log.Error(err, "Failed to add manager factory to controller.")
diff --git a/internal/helm/controller/controller.go b/internal/helm/controller/controller.go
@@ -52,6 +52,7 @@ type WatchOptions struct {
 	SuppressOverrideValues  bool
 	MaxConcurrentReconciles int
 	Selector                metav1.LabelSelector
+	DryRunOption            string
 }
 
 // Add creates a new helm operator controller and adds it to the manager
@@ -66,6 +67,7 @@ func Add(mgr manager.Manager, options WatchOptions) error {
 		ReconcilePeriod:        options.ReconcilePeriod,
 		OverrideValues:         options.OverrideValues,
 		SuppressOverrideValues: options.SuppressOverrideValues,
+		DryRunOption:           options.DryRunOption,
 	}
 
 	c, err := controller.New(controllerName, mgr, controller.Options{
diff --git a/internal/helm/controller/reconcile.go b/internal/helm/controller/reconcile.go
@@ -55,6 +55,7 @@ type HelmOperatorReconciler struct {
 	OverrideValues         map[string]string
 	SuppressOverrideValues bool
 	releaseHook            ReleaseHookFunc
+	DryRunOption           string
 }
 
 const (
@@ -96,7 +97,7 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile
 		return reconcile.Result{}, err
 	}
 
-	manager, err := r.ManagerFactory.NewManager(o, r.OverrideValues)
+	manager, err := r.ManagerFactory.NewManager(o, r.OverrideValues, r.DryRunOption)
 	if err != nil {
 		log.Error(err, "Failed to get release manager")
 		return reconcile.Result{}, err
diff --git a/internal/helm/release/manager.go b/internal/helm/release/manager.go
@@ -75,6 +75,8 @@ type manager struct {
 	isUpgradeRequired bool
 	deployedRelease   *rpb.Release
 	chart             *cpb.Chart
+
+	dryRunOption string
 }
 
 type InstallOption func(*action.Install) error
@@ -159,6 +161,7 @@ func (m manager) getCandidateRelease(namespace, name string, chart *cpb.Chart,
 	upgrade := action.NewUpgrade(m.actionConfig)
 	upgrade.Namespace = namespace
 	upgrade.DryRun = true
+	upgrade.DryRunOption = m.dryRunOption
 	return upgrade.Run(name, chart, values)
 }
 
diff --git a/internal/helm/release/manager_factory.go b/internal/helm/release/manager_factory.go
@@ -33,7 +33,7 @@ import (
 // improves decoupling between reconciliation logic and the Helm backend
 // components used to manage releases.
 type ManagerFactory interface {
-	NewManager(r *unstructured.Unstructured, overrideValues map[string]string) (Manager, error)
+	NewManager(r *unstructured.Unstructured, overrideValues map[string]string, dryRunOption string) (Manager, error)
 }
 
 type managerFactory struct {
@@ -47,7 +47,7 @@ func NewManagerFactory(mgr crmanager.Manager, acg client.ActionConfigGetter, cha
 	return &managerFactory{mgr, acg, chartDir}
 }
 
-func (f managerFactory) NewManager(cr *unstructured.Unstructured, overrideValues map[string]string) (Manager, error) {
+func (f managerFactory) NewManager(cr *unstructured.Unstructured, overrideValues map[string]string, dryRunOption string) (Manager, error) {
 	actionConfig, err := f.acg.ActionConfigFor(cr)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get helm action config: %w", err)
@@ -86,9 +86,10 @@ func (f managerFactory) NewManager(cr *unstructured.Unstructured, overrideValues
 		releaseName: releaseName,
 		namespace:   cr.GetNamespace(),
 
-		chart:  crChart,
-		values: values,
-		status: types.StatusFor(cr),
+		chart:        crChart,
+		values:       values,
+		status:       types.StatusFor(cr),
+		dryRunOption: dryRunOption,
 	}, nil
 }
 
diff --git a/internal/helm/watches/watches.go b/internal/helm/watches/watches.go
@@ -40,6 +40,7 @@ type Watch struct {
 	OverrideValues          map[string]string    `json:"overrideValues,omitempty"`
 	Selector                metav1.LabelSelector `json:"selector"`
 	ReconcilePeriod         metav1.Duration      `json:"reconcilePeriod,omitempty"`
+	DryRunOption            string               `json:"dryRunOption,omitempty"`
 }
 
 // UnmarshalYAML unmarshals an individual watch from the Helm watches.yaml file
diff --git a/internal/helm/watches/watches_test.go b/internal/helm/watches/watches_test.go
@@ -101,7 +101,30 @@ func TestLoadReader(t *testing.T) {
 			},
 			expectErr: false,
 		},
-
+		{
+			name: "valid with dry run option",
+			data: `---
+- group: mygroup
+  version: v1alpha1
+  kind: MyKind
+  chart: ../../../internal/plugins/helm/v1/chartutil/testdata/test-chart
+  watchDependentResources: false
+  overrideValues:
+    key: $MY_VALUE
+  dryRunOption: server
+`,
+			env: map[string]string{"MY_VALUE": "value"},
+			expectWatches: []Watch{
+				{
+					GroupVersionKind:        schema.GroupVersionKind{Group: "mygroup", Version: "v1alpha1", Kind: "MyKind"},
+					ChartDir:                "../../../internal/plugins/helm/v1/chartutil/testdata/test-chart",
+					WatchDependentResources: &falseVal,
+					OverrideValues:          map[string]string{"key": "value"},
+					DryRunOption:            "server",
+				},
+			},
+			expectErr: false,
+		},
 		{
 			name: "invalid with override template expansion",
 			data: `---
diff --git a/testdata/go/v4/memcached-operator/Makefile b/testdata/go/v4/memcached-operator/Makefile
@@ -48,7 +48,7 @@ endif
 
 # Set the Operator SDK version to use. By default, what is installed on the system is used.
 # This is useful for CI or a project to utilize a specific version of the operator-sdk toolkit.
-OPERATOR_SDK_VERSION ?= v1.33.0
+OPERATOR_SDK_VERSION ?= v1.34.0
 
 # Image URL to use all building/pushing image targets
 IMG ?= controller:latest
diff --git a/testdata/go/v4/monitoring/memcached-operator/Makefile b/testdata/go/v4/monitoring/memcached-operator/Makefile
@@ -48,7 +48,7 @@ endif
 
 # Set the Operator SDK version to use. By default, what is installed on the system is used.
 # This is useful for CI or a project to utilize a specific version of the operator-sdk toolkit.
-OPERATOR_SDK_VERSION ?= v1.33.0
+OPERATOR_SDK_VERSION ?= v1.34.0
 
 # Image URL to use all building/pushing image targets
 IMG ?= controller:latest
diff --git a/testdata/helm/memcached-operator/Makefile b/testdata/helm/memcached-operator/Makefile
@@ -48,7 +48,7 @@ endif
 
 # Set the Operator SDK version to use. By default, what is installed on the system is used.
 # This is useful for CI or a project to utilize a specific version of the operator-sdk toolkit.
-OPERATOR_SDK_VERSION ?= v1.33.0
+OPERATOR_SDK_VERSION ?= v1.34.0
 
 # Image URL to use all building/pushing image targets
 IMG ?= controller:latest
diff --git a/website/content/en/docs/best-practices/common-recommendation.md b/website/content/en/docs/best-practices/common-recommendation.md
@@ -105,4 +105,4 @@ spec:
 [k8s-manage-resources]: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/
 [best practices]: https://olm.operatorframework.io/docs/concepts/olm-architecture/dependency-resolution/
 [Dependency Resolution]:  /docs/best-practices/best-practices
-[sample]: https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v3/memcached-operator
+[sample]: https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v4/memcached-operator
diff --git a/website/content/en/docs/best-practices/pod-security-standards.md b/website/content/en/docs/best-practices/pod-security-standards.md
@@ -134,7 +134,7 @@ the Operator bundle generated with the target is built from the manifests under
 the layout of your operator built with Operator-SDK see [Project Layout][project-layout].
 
 To check an example of a CSV which complies with the [restricted][restricted] policy, see the Golang sample
-under the [testdata/go/v3/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml](https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/v3/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml)
+under the [testdata/go/v4/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml](https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/v4/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml)
 
 ### How can I verify my manifest? 
 
@@ -419,5 +419,5 @@ Therefore, if you want your workloads running in namespaces labeled to enforce r
 [restricted]: https://kubernetes.io/docs/concepts/security/pod-security-standards/#restricted
 [security-standards]: https://kubernetes.io/docs/concepts/security/pod-security-standards/
 [psachecker]: https://github.com/stlaz/psachecker
-[sample]: https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v3/memcached-operator
-[docker-good-practices-doc]: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#user
+[sample]: https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v4/memcached-operator
+[docker-good-practices-doc]: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#user
diff --git a/website/content/en/docs/building-operators/golang/migration.md b/website/content/en/docs/building-operators/golang/migration.md
@@ -418,7 +418,7 @@ For further steps regarding the deployment of the operator, creation of custom r
 [install-guide]: /docs/building-operators/ansible/installation
 [image-reg-config]:/docs/olm-integration/cli-overview#private-bundle-and-catalog-image-registries
 [metrics]: https://book.kubebuilder.io/reference/metrics.html?highlight=metr#metrics
-[memcached_controller]: https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v3/memcached-operator
+[memcached_controller]: https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v4/memcached-operator
 [rbac_markers]: https://book.kubebuilder.io/reference/markers/rbac.html
 [kube-auth-proxy]: https://github.com/brancz/kube-rbac-proxy
 [markers]: https://book.kubebuilder.io/reference/markers.html?highlight=markers#marker-syntax
diff --git a/website/content/en/docs/building-operators/golang/references/client.md b/website/content/en/docs/building-operators/golang/references/client.md
@@ -580,7 +580,7 @@ func labelsForApp(name string) map[string]string {
 }
 ```
 
-[memcached-testdata]:https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v3/memcached-operator
+[memcached-testdata]:https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v4/memcached-operator
 [repo-controller-runtime]:https://github.com/kubernetes-sigs/controller-runtime
 [doc-client]:https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/client#Client
 [doc-split-client]:https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/client#DelegatingClient
diff --git a/website/content/en/docs/building-operators/golang/testing.md b/website/content/en/docs/building-operators/golang/testing.md
@@ -54,7 +54,7 @@ To implement application-specific tests, the SDK's test harness, [scorecard][sco
 [scorecard]: /docs/testing-operators/scorecard/
 [gomega]: https://onsi.github.io/gomega/
 [kuttl]: https://kuttl.dev/
-[sample]: https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v3/memcached-operator
+[sample]: https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v4/memcached-operator
 [molecule]: https://molecule.readthedocs.io/
 [molecule-tests]: /docs/building-operators/ansible/testing-guide
 [helm-chart-tests]: https://helm.sh/docs/topics/chart_tests/
diff --git a/website/content/en/docs/building-operators/golang/tutorial.md b/website/content/en/docs/building-operators/golang/tutorial.md
@@ -543,7 +543,7 @@ Next, check out the following:
 [legacy_CLI]:https://github.com/operator-framework/operator-sdk/tree/v0.19.x/website/content/en/docs/cli
 [manager_go_doc]: https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/manager#Manager
 [markers]: https://book.kubebuilder.io/reference/markers.html
-[memcached_controller]: https://github.com/operator-framework/operator-sdk/blob/latest/testdata/go/v3/memcached-operator/controllers/memcached_controller.go
+[memcached_controller]: https://github.com/operator-framework/operator-sdk/blob/latest/testdata/go/v4/memcached-operator/internal/controller/memcached_controller.go
 [migration-guide]:/docs/building-operators/golang/migration
 [multi-namespaced-cache-builder]: https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/cache#MultiNamespacedCacheBuilder
 [multigroup-kubebuilder-doc]: https://book.kubebuilder.io/migration/multi-group.html
diff --git a/website/content/en/docs/testing-operators/scorecard/custom-tests.md b/website/content/en/docs/testing-operators/scorecard/custom-tests.md
@@ -311,7 +311,7 @@ connection to invoke the Kube API.
 [basic_tests]: https://github.com/operator-framework/operator-sdk/blob/09c3aa14625965af9f22f513cd5c891471dbded2/internal/scorecard/tests/basic.go
 [config_yaml]: https://github.com/operator-framework/operator-sdk/blob/09c3aa14625965af9f22f513cd5c891471dbded2/internal/scorecard/testdata/bundle/tests/scorecard/config.yaml
 [scorecard_main_func]: https://github.com/operator-framework/operator-sdk/blob/09c3aa14625965af9f22f513cd5c891471dbded2/images/scorecard-test/main.go
-[custom_scorecard_repo]:https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v3/memcached-operator 
+[custom_scorecard_repo]:https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v4/memcached-operator 
 [user_doc]: /docs/testing-operators/scorecard/
 [scorecard_binary]: https://github.com/operator-framework/operator-sdk/blob/09c3aa14625965af9f22f513cd5c891471dbded2/images/custom-scorecard-tests/main.go
 [sample_makefile]: https://github.com/operator-framework/operator-sdk/blob/09c3aa14625965af9f22f513cd5c891471dbded2/Makefile

Original file line number	Diff line number	Diff line change
`@@ -52,6 +52,7 @@ type WatchOptions struct {`
`52`	`52`	`SuppressOverrideValues bool`
`53`	`53`	`MaxConcurrentReconciles int`
`54`	`54`	`Selector metav1.LabelSelector`
	`55`	`+ DryRunOption string`
`55`	`56`	`}`
`56`	`57`
`57`	`58`	`// Add creates a new helm operator controller and adds it to the manager`
`@@ -66,6 +67,7 @@ func Add(mgr manager.Manager, options WatchOptions) error {`
`66`	`67`	`ReconcilePeriod: options.ReconcilePeriod,`
`67`	`68`	`OverrideValues: options.OverrideValues,`
`68`	`69`	`SuppressOverrideValues: options.SuppressOverrideValues,`
	`70`	`+ DryRunOption: options.DryRunOption,`
`69`	`71`	`}`
`70`	`72`
`71`	`73`	`c, err := controller.New(controllerName, mgr, controller.Options{`
Original file line number	Diff line number	Diff line change
`@@ -55,6 +55,7 @@ type HelmOperatorReconciler struct {`
`55`	`55`	`OverrideValues map[string]string`
`56`	`56`	`SuppressOverrideValues bool`
`57`	`57`	`releaseHook ReleaseHookFunc`
	`58`	`+ DryRunOption string`
`58`	`59`	`}`
`59`	`60`
`60`	`61`	`const (`
`@@ -96,7 +97,7 @@ func (r HelmOperatorReconciler) Reconcile(ctx context.Context, request reconcile`
`96`	`97`	`return reconcile.Result{}, err`
`97`	`98`	`}`
`98`	`99`
`99`		`- manager, err := r.ManagerFactory.NewManager(o, r.OverrideValues)`
	`100`	`+ manager, err := r.ManagerFactory.NewManager(o, r.OverrideValues, r.DryRunOption)`
`100`	`101`	`if err != nil {`
`101`	`102`	`log.Error(err, "Failed to get release manager")`
`102`	`103`	`return reconcile.Result{}, err`
Original file line number	Diff line number	Diff line change
`@@ -75,6 +75,8 @@ type manager struct {`
`75`	`75`	`isUpgradeRequired bool`
`76`	`76`	`deployedRelease *rpb.Release`
`77`	`77`	`chart *cpb.Chart`
	`78`	`+`
	`79`	`+ dryRunOption string`
`78`	`80`	`}`
`79`	`81`
`80`	`82`	`type InstallOption func(*action.Install) error`
`@@ -159,6 +161,7 @@ func (m manager) getCandidateRelease(namespace, name string, chart *cpb.Chart,`
`159`	`161`	`upgrade := action.NewUpgrade(m.actionConfig)`
`160`	`162`	`upgrade.Namespace = namespace`
`161`	`163`	`upgrade.DryRun = true`
	`164`	`+ upgrade.DryRunOption = m.dryRunOption`
`162`	`165`	`return upgrade.Run(name, chart, values)`
`163`	`166`	`}`
`164`	`167`
Original file line number	Diff line number	Diff line change
`@@ -40,6 +40,7 @@ type Watch struct {`
`40`	`40`	OverrideValues map[string]string `json:"overrideValues,omitempty"`
`41`	`41`	Selector metav1.LabelSelector `json:"selector"`
`42`	`42`	ReconcilePeriod metav1.Duration `json:"reconcilePeriod,omitempty"`
	`43`	+ DryRunOption string `json:"dryRunOption,omitempty"`
`43`	`44`	`}`
`44`	`45`
`45`	`46`	`// UnmarshalYAML unmarshals an individual watch from the Helm watches.yaml file`
Original file line number	Diff line number	Diff line change
`@@ -580,7 +580,7 @@ func labelsForApp(name string) map[string]string {`
`580`	`580`	`}`
`581`	`581`	```
`582`	`582`
`583`		`-[memcached-testdata]:https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v3/memcached-operator`
	`583`	`+[memcached-testdata]:https://github.com/operator-framework/operator-sdk/tree/master/testdata/go/v4/memcached-operator`
`584`	`584`	`[repo-controller-runtime]:https://github.com/kubernetes-sigs/controller-runtime`
`585`	`585`	`[doc-client]:https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/client#Client`
`586`	`586`	`[doc-split-client]:https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/client#DelegatingClient`