diff --git a/internal/cmd/helm-operator/run/cmd.go b/internal/cmd/helm-operator/run/cmd.go index 58af47d7..b6082335 100644 --- a/internal/cmd/helm-operator/run/cmd.go +++ b/internal/cmd/helm-operator/run/cmd.go @@ -23,14 +23,12 @@ import ( "strings" "github.com/operator-framework/helm-operator-plugins/internal/flags" - watches "github.com/operator-framework/helm-operator-plugins/internal/legacy/watches" "github.com/operator-framework/helm-operator-plugins/internal/metrics" "github.com/operator-framework/helm-operator-plugins/internal/version" "github.com/operator-framework/helm-operator-plugins/pkg/annotation" helmmgr "github.com/operator-framework/helm-operator-plugins/pkg/manager" "github.com/operator-framework/helm-operator-plugins/pkg/reconciler" - "helm.sh/helm/v3/pkg/chart" - "helm.sh/helm/v3/pkg/chart/loader" + "github.com/operator-framework/helm-operator-plugins/pkg/watches" "github.com/spf13/cobra" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -179,18 +177,11 @@ func run(cmd *cobra.Command, f *flags.Flags) { for _, w := range ws { - // TODO: remove this after modifying watches of hybrid lib. - cl, err := getChart(w) - if err != nil { - log.Error(err, "Unable to read chart") - os.Exit(1) - } - r, err := reconciler.New( - reconciler.WithChart(*cl), + reconciler.WithChart(*w.Chart), reconciler.WithGroupVersionKind(w.GroupVersionKind), reconciler.WithOverrideValues(w.OverrideValues), - reconciler.WithSelector(w.Selector), + reconciler.WithSelector(*w.Selector), reconciler.SkipDependentWatches(*w.WatchDependentResources), reconciler.WithMaxConcurrentReconciles(f.MaxConcurrentReconciles), reconciler.WithReconcilePeriod(f.ReconcilePeriod), @@ -207,7 +198,7 @@ func run(cmd *cobra.Command, f *flags.Flags) { log.Error(err, "unable to create controller", "Helm") os.Exit(1) } - log.Info("configured watch", "gvk", w.GroupVersionKind, "chartDir", w.ChartDir, "maxConcurrentReconciles", f.MaxConcurrentReconciles, "reconcilePeriod", f.ReconcilePeriod) + log.Info("configured watch", "gvk", w.GroupVersionKind, "chartDir", w.ChartPath, "maxConcurrentReconciles", f.MaxConcurrentReconciles, "reconcilePeriod", f.ReconcilePeriod) } log.Info("starting manager") @@ -239,13 +230,3 @@ func exitIfUnsupported(options manager.Options) { os.Exit(1) } } - -// getChart returns the chart from the chartDir passed to the watches file. -func getChart(w watches.Watch) (*chart.Chart, error) { - c, err := loader.LoadDir(w.ChartDir) - if err != nil { - return nil, fmt.Errorf("failed to load chart dir: %w", err) - } - - return c, nil -} diff --git a/internal/cmd/hybrid-operator/run/cmd.go b/internal/cmd/hybrid-operator/run/cmd.go index 46ca379b..feebbcc7 100644 --- a/internal/cmd/hybrid-operator/run/cmd.go +++ b/internal/cmd/hybrid-operator/run/cmd.go @@ -168,6 +168,7 @@ func run(cmd *cobra.Command, f *flags.Flags) { reconciler.WithChart(*w.Chart), reconciler.WithGroupVersionKind(w.GroupVersionKind), reconciler.WithOverrideValues(w.OverrideValues), + reconciler.WithSelector(*w.Selector), reconciler.SkipDependentWatches(w.WatchDependentResources != nil && !*w.WatchDependentResources), reconciler.WithMaxConcurrentReconciles(maxConcurrentReconciles), reconciler.WithReconcilePeriod(reconcilePeriod), diff --git a/internal/legacy/watches/watches.go b/internal/legacy/watches/watches.go deleted file mode 100644 index e2f43a1d..00000000 --- a/internal/legacy/watches/watches.go +++ /dev/null @@ -1,159 +0,0 @@ -// Copyright 2019 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package watches - -import ( - "bytes" - "errors" - "fmt" - "io" - "io/ioutil" - "os" - "text/template" - - sprig "github.com/go-task/slim-sprig" - "helm.sh/helm/v3/pkg/chartutil" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/yaml" -) - -const WatchesFile = "watches.yaml" - -// Watch defines options for configuring a watch for a Helm-based -// custom resource. -type Watch struct { - schema.GroupVersionKind `json:",inline"` - ChartDir string `json:"chart"` - WatchDependentResources *bool `json:"watchDependentResources,omitempty"` - OverrideValues map[string]string `json:"overrideValues,omitempty"` - Selector metav1.LabelSelector `json:"selector"` -} - -// UnmarshalYAML unmarshals an individual watch from the Helm watches.yaml file -// into a Watch struct. -// -// Deprecated: This function is no longer used internally to unmarshal -// watches.yaml data. To ensure the correct defaults are applied when loading -// watches.yaml, use Load() or LoadReader() instead of this function and/or -// yaml.Unmarshal(). -func (w *Watch) UnmarshalYAML(unmarshal func(interface{}) error) error { - // by default, the operator will watch dependent resources - trueVal := true - w.WatchDependentResources = &trueVal - - // hide watch data in plain struct to prevent unmarshal from calling - // UnmarshalYAML again - type plain Watch - - return unmarshal((*plain)(w)) -} - -// Load loads a slice of Watches from the watch file at `path`. For each entry -// in the watches file, it verifies the configuration. If an error is -// encountered loading the file or verifying the configuration, it will be -// returned. -func Load(path string) ([]Watch, error) { - f, err := os.Open(path) - if err != nil { - return nil, fmt.Errorf("could not open watches file: %w", err) - } - w, err := LoadReader(f) - - // Make sure to close the file, regardless of the error returned by - // LoadReader. - if err := f.Close(); err != nil { - return nil, fmt.Errorf("could not close watches file: %w", err) - } - return w, err -} - -// LoadReader loads a slice of Watches from the provided reader. For each entry -// in the watches file, it verifies the configuration. If an error is -// encountered reading or verifying the configuration, it will be returned. -func LoadReader(reader io.Reader) ([]Watch, error) { - b, err := ioutil.ReadAll(reader) - if err != nil { - return nil, err - } - - watches := []Watch{} - err = yaml.Unmarshal(b, &watches) - if err != nil { - return nil, err - } - - watchesMap := make(map[schema.GroupVersionKind]struct{}) - for i, w := range watches { - gvk := w.GroupVersionKind - - if err := verifyGVK(gvk); err != nil { - return nil, fmt.Errorf("invalid GVK: %s: %w", gvk, err) - } - - if _, err := chartutil.IsChartDir(w.ChartDir); err != nil { - return nil, fmt.Errorf("invalid chart directory %s: %w", w.ChartDir, err) - } - - if _, ok := watchesMap[gvk]; ok { - return nil, fmt.Errorf("duplicate GVK: %s", gvk) - } - watchesMap[gvk] = struct{}{} - if w.WatchDependentResources == nil { - trueVal := true - w.WatchDependentResources = &trueVal - } - w.OverrideValues, err = expandOverrideValues(w.OverrideValues) - if err != nil { - return nil, fmt.Errorf("failed to expand override values: %v", err) - } - watches[i] = w - } - return watches, nil -} - -func expandOverrideValues(in map[string]string) (map[string]string, error) { - if in == nil { - return nil, nil - } - out := make(map[string]string) - for k, v := range in { - envV := os.ExpandEnv(v) - - v := &bytes.Buffer{} - tmplV, err := template.New(k).Funcs(sprig.TxtFuncMap()).Parse(envV) - if err != nil { - return nil, fmt.Errorf("invalid template string %q: %v", envV, err) - } - if err := tmplV.Execute(v, nil); err != nil { - return nil, fmt.Errorf("failed to execute template %q: %v", envV, err) - } - out[k] = v.String() - } - return out, nil -} - -func verifyGVK(gvk schema.GroupVersionKind) error { - // A GVK without a group is valid. Certain scenarios may cause a GVK - // without a group to fail in other ways later in the initialization - // process. - if gvk.Version == "" { - return errors.New("version must not be empty") - } - if gvk.Kind == "" { - return errors.New("kind must not be empty") - } - return nil -} diff --git a/internal/legacy/watches/watches_test.go b/internal/legacy/watches/watches_test.go deleted file mode 100644 index 5e854e7f..00000000 --- a/internal/legacy/watches/watches_test.go +++ /dev/null @@ -1,310 +0,0 @@ -// Copyright 2019 The Operator-SDK Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package watches - -import ( - "bytes" - "io/ioutil" - "os" - "testing" - - "github.com/stretchr/testify/assert" - "k8s.io/apimachinery/pkg/runtime/schema" -) - -func TestLoadReader(t *testing.T) { - trueVal, falseVal := true, false - testCases := []struct { - name string - data string - env map[string]string - expectWatches []Watch - expectErr bool - }{ - { - name: "valid", - data: `--- -- group: mygroup - version: v1alpha1 - kind: MyKind - chart: ../../../pkg/internal/testdata/test-chart - watchDependentResources: false - overrideValues: - key: value -`, - expectWatches: []Watch{ - { - GroupVersionKind: schema.GroupVersionKind{Group: "mygroup", Version: "v1alpha1", Kind: "MyKind"}, - ChartDir: "../../../pkg/internal/testdata/test-chart", - WatchDependentResources: &falseVal, - OverrideValues: map[string]string{"key": "value"}, - }, - }, - expectErr: false, - }, - { - name: "valid with override env expansion", - data: `--- -- group: mygroup - version: v1alpha1 - kind: MyKind - chart: ../../../pkg/internal/testdata/test-chart - watchDependentResources: false - overrideValues: - key: $MY_VALUE -`, - env: map[string]string{"MY_VALUE": "value"}, - expectWatches: []Watch{ - { - GroupVersionKind: schema.GroupVersionKind{Group: "mygroup", Version: "v1alpha1", Kind: "MyKind"}, - ChartDir: "../../../pkg/internal/testdata/test-chart", - WatchDependentResources: &falseVal, - OverrideValues: map[string]string{"key": "value"}, - }, - }, - expectErr: false, - }, - { - name: "valid with override template expansion", - data: `--- -- group: mygroup - version: v1alpha1 - kind: MyKind - chart: ../../../pkg/internal/testdata/test-chart - watchDependentResources: false - overrideValues: - repo: '{{ ("$MY_IMAGE" | split ":")._0 }}' - tag: '{{ ("$MY_IMAGE" | split ":")._1 }}' -`, - env: map[string]string{"MY_IMAGE": "quay.io/operator-framework/helm-operator:latest"}, - expectWatches: []Watch{ - { - GroupVersionKind: schema.GroupVersionKind{Group: "mygroup", Version: "v1alpha1", Kind: "MyKind"}, - ChartDir: "../../../pkg/internal/testdata/test-chart", - WatchDependentResources: &falseVal, - OverrideValues: map[string]string{ - "repo": "quay.io/operator-framework/helm-operator", - "tag": "latest", - }, - }, - }, - expectErr: false, - }, - - { - name: "invalid with override template expansion", - data: `--- -- group: mygroup - version: v1alpha1 - kind: MyKind - chart: ../../../pkg/internal/testdata/test-chart - watchDependentResources: false - overrideValues: - repo: '{{ ("$MY_IMAGE" | split ":")._0 }}' - tag: '{{ ("$MY_IMAGE" | split ":")._1' -`, - env: map[string]string{"MY_IMAGE": "quay.io/operator-framework/helm-operator:latest"}, - expectErr: true, - }, - { - name: "multiple gvk", - data: `--- -- group: mygroup - version: v1alpha1 - kind: MyFirstKind - chart: ../../../pkg/internal/testdata/test-chart -- group: mygroup - version: v1alpha1 - kind: MySecondKind - chart: ../../../pkg/internal/testdata/test-chart -`, - expectWatches: []Watch{ - { - GroupVersionKind: schema.GroupVersionKind{Group: "mygroup", Version: "v1alpha1", Kind: "MyFirstKind"}, - ChartDir: "../../../pkg/internal/testdata/test-chart", - WatchDependentResources: &trueVal, - }, - { - GroupVersionKind: schema.GroupVersionKind{Group: "mygroup", Version: "v1alpha1", Kind: "MySecondKind"}, - ChartDir: "../../../pkg/internal/testdata/test-chart", - WatchDependentResources: &trueVal, - }, - }, - expectErr: false, - }, - { - name: "duplicate gvk", - data: `--- -- group: mygroup - version: v1alpha1 - kind: MyKind - chart: ../../../pkg/internal/testdata/test-chart -- group: mygroup - version: v1alpha1 - kind: MyKind - chart: ../../../pkg/internal/testdata/test-chart -`, - expectErr: true, - }, - { - name: "no version", - data: `--- -- group: mygroup - kind: MyKind - chart: ../../../pkg/internal/testdata/test-chart -`, - expectErr: true, - }, - { - name: "no kind", - data: `--- -- group: mygroup - version: v1alpha1 - chart: ../../../pkg/internal/testdata/test-chart -`, - expectErr: true, - }, - { - name: "bad chart path", - data: `--- -- group: mygroup - version: v1alpha1 - kind: MyKind - chart: nonexistent/path/to/chart -`, - expectErr: true, - }, - { - name: "invalid overrides", - data: `--- -- group: mygroup - version: v1alpha1 - kind: MyKind - chart: ../../../pkg/internal/testdata/test-chart - overrideValues: - key1: - key2: value -`, - expectErr: true, - }, - { - name: "invalid yaml", - data: `--- -foo: bar -`, - expectErr: true, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - for k, v := range tc.env { - if err := os.Setenv(k, v); err != nil { - t.Fatalf("Failed to set environment variable %q: %v", k, err) - } - } - - watchesData := bytes.NewBufferString(tc.data) - watches, err := LoadReader(watchesData) - if !tc.expectErr && err != nil { - t.Fatalf("Expected no error; got error: %v", err) - } else if tc.expectErr && err == nil { - t.Fatalf("Expected error; got no error") - } - assert.Equal(t, tc.expectWatches, watches) - - for k := range tc.env { - if err := os.Unsetenv(k); err != nil { - t.Fatalf("Failed to unset environment variable %q: %v", k, err) - } - } - }) - } -} - -func TestLoad(t *testing.T) { - falseVal := false - testCases := []struct { - name string - data string - env map[string]string - expectWatches []Watch - expectErr bool - }{ - { - name: "valid", - data: `--- -- group: mygroup - version: v1alpha1 - kind: MyKind - chart: ../../../pkg/internal/testdata/test-chart - watchDependentResources: false - overrideValues: - key: value -`, - expectWatches: []Watch{ - { - GroupVersionKind: schema.GroupVersionKind{Group: "mygroup", Version: "v1alpha1", Kind: "MyKind"}, - ChartDir: "../../../pkg/internal/testdata/test-chart", - WatchDependentResources: &falseVal, - OverrideValues: map[string]string{"key": "value"}, - }, - }, - expectErr: false, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - for k, v := range tc.env { - if err := os.Setenv(k, v); err != nil { - t.Fatalf("Failed to set environment variable %q: %v", k, err) - } - } - - f, err := ioutil.TempFile("", "osdk-test-load") - if err != nil { - t.Fatalf("Failed to create temporary watches file: %v", err) - } - defer removeFile(t, f) - if _, err := f.WriteString(tc.data); err != nil { - t.Fatalf("Failed to write temporary watches file: %v", err) - } - watches, err := Load(f.Name()) - if !tc.expectErr && err != nil { - t.Fatalf("Expected no error; got error: %v", err) - } else if tc.expectErr && err == nil { - t.Fatalf("Expected error; got no error") - } - assert.Equal(t, tc.expectWatches, watches) - - for k := range tc.env { - if err := os.Unsetenv(k); err != nil { - t.Fatalf("Failed to unset environment variable %q: %v", k, err) - } - } - }) - } -} - -// remove removes path from disk. Used in defer statements. -func removeFile(t *testing.T, f *os.File) { - if err := f.Close(); err != nil { - t.Fatal(err) - } - if err := os.Remove(f.Name()); err != nil { - t.Fatal(err) - } -} diff --git a/pkg/watches/watches.go b/pkg/watches/watches.go index 7f408422..0132ec89 100644 --- a/pkg/watches/watches.go +++ b/pkg/watches/watches.go @@ -35,11 +35,12 @@ type Watch struct { schema.GroupVersionKind `json:",inline"` ChartPath string `json:"chart"` - WatchDependentResources *bool `json:"watchDependentResources,omitempty"` - OverrideValues map[string]string `json:"overrideValues,omitempty"` - ReconcilePeriod *metav1.Duration `json:"reconcilePeriod,omitempty"` - MaxConcurrentReconciles *int `json:"maxConcurrentReconciles,omitempty"` - Chart *chart.Chart `json:"-"` + WatchDependentResources *bool `json:"watchDependentResources,omitempty"` + OverrideValues map[string]string `json:"overrideValues,omitempty"` + ReconcilePeriod *metav1.Duration `json:"reconcilePeriod,omitempty"` + MaxConcurrentReconciles *int `json:"maxConcurrentReconciles,omitempty"` + Selector *metav1.LabelSelector `json:"selector,omitempty"` + Chart *chart.Chart `json:"-"` } // Load loads a slice of Watches from the watch file at `path`. For each entry diff --git a/pkg/watches/watches_test.go b/pkg/watches/watches_test.go index 4526d41e..dec09044 100644 --- a/pkg/watches/watches_test.go +++ b/pkg/watches/watches_test.go @@ -38,6 +38,9 @@ var _ = Describe("LoadReader", func() { kind: MyKind chart: ../../pkg/internal/testdata/test-chart watchDependentResources: false + selector: + matchExpressions: + - {key: testLabel, operator: Exists, values: []} overrideValues: key: value ` @@ -47,6 +50,14 @@ var _ = Describe("LoadReader", func() { ChartPath: "../../pkg/internal/testdata/test-chart", WatchDependentResources: &falseVal, OverrideValues: map[string]string{"key": "value"}, + Selector: &v1.LabelSelector{ + MatchLabels: nil, + MatchExpressions: []v1.LabelSelectorRequirement{{ + Key: "testLabel", + Operator: v1.LabelSelectorOpExists, + Values: []string{}, + }}, + }, }, } @@ -329,6 +340,7 @@ func verifyEqualWatches(expectedWatch, obtainedWatch []Watch) { Expect(expectedWatch[i].OverrideValues).To(BeEquivalentTo(obtainedWatch[i].OverrideValues)) Expect(expectedWatch[i].MaxConcurrentReconciles).To(BeEquivalentTo(obtainedWatch[i].MaxConcurrentReconciles)) Expect(expectedWatch[i].ReconcilePeriod).To(BeEquivalentTo(obtainedWatch[i].ReconcilePeriod)) + Expect(expectedWatch[i].Selector).To(BeEquivalentTo(obtainedWatch[i].Selector)) } }