From 4cd2f8263d678fcc330269b0e38c87b907d77418 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Fri, 21 Apr 2023 09:25:53 -0400 Subject: [PATCH] Handle channelName (#172) Adds backend support for channelName Fixes #170 Fixes OPRUN-2917 * Integrate channelName * Add channelName unit tests * Update error messages * Update channel name regex Signed-off-by: Todd Short --- .gitignore | 2 + api/v1alpha1/operator_types.go | 2 +- ...rators.operatorframework.io_operators.yaml | 2 +- controllers/admission_test.go | 45 ++++ controllers/operator_controller_test.go | 201 +++++++++++++++++- .../resolution/variable_sources/olm/olm.go | 3 + .../required_package/required_package.go | 17 ++ 7 files changed, 269 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index e48b81835..2d49c9a2d 100644 --- a/.gitignore +++ b/.gitignore @@ -30,6 +30,8 @@ install.sh *.swp *.swo *~ +\#*\# +.\#* # TODO dfranz remove this line and the bin folder when tools binaries are moved to their own folder !bin/.dockerignore diff --git a/api/v1alpha1/operator_types.go b/api/v1alpha1/operator_types.go index d0ab38f28..409abc5bd 100644 --- a/api/v1alpha1/operator_types.go +++ b/api/v1alpha1/operator_types.go @@ -39,7 +39,7 @@ type OperatorSpec struct { Version string `json:"version,omitempty"` //+kubebuilder:validation:MaxLength:=48 - //+kubebuilder:validation:Pattern:=^[a-z0-9]+(-[a-z0-9]+)*$ + //+kubebuilder:validation:Pattern:=^[a-z0-9]+([\.-][a-z0-9]+)*$ // Channel constraint defintion Channel string `json:"channel,omitempty"` } diff --git a/config/crd/bases/operators.operatorframework.io_operators.yaml b/config/crd/bases/operators.operatorframework.io_operators.yaml index 759934b44..6cce301f7 100644 --- a/config/crd/bases/operators.operatorframework.io_operators.yaml +++ b/config/crd/bases/operators.operatorframework.io_operators.yaml @@ -38,7 +38,7 @@ spec: channel: description: Channel constraint defintion maxLength: 48 - pattern: ^[a-z0-9]+(-[a-z0-9]+)*$ + pattern: ^[a-z0-9]+([\.-][a-z0-9]+)*$ type: string packageName: maxLength: 48 diff --git a/controllers/admission_test.go b/controllers/admission_test.go index 9e3414624..ae568d019 100644 --- a/controllers/admission_test.go +++ b/controllers/admission_test.go @@ -72,4 +72,49 @@ var _ = Describe("Operator Spec Validations", func() { Expect(err.Error()).To(ContainSubstring("spec.version in body should match '^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(-(0|[1-9]\\d*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*)(\\.(0|[1-9]\\d*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*))*)?(\\+([0-9a-zA-Z-]+(\\.[0-9a-zA-Z-]+)*))?$'")) } }) + It("should fail if an invalid channel name is given", func() { + invalidChannels := []string{ + "spaces spaces", + "Capitalized", + "camelCase", + "many/invalid$characters+in_name", + "-start-with-hyphen", + "end-with-hyphen-", + ".start-with-period", + "end-with-period.", + } + for _, invalidChannel := range invalidChannels { + err := cl.Create(ctx, operator(operatorsv1alpha1.OperatorSpec{ + PackageName: "package", + Channel: invalidChannel, + })) + Expect(err).To(HaveOccurred(), "expected error for invalid channel '%q'", invalidChannel) + Expect(err.Error()).To(ContainSubstring("spec.channel in body should match '^[a-z0-9]+([\\.-][a-z0-9]+)*$'")) + } + }) + It("should pass if a valid channel name is given", func() { + validChannels := []string{ + "hyphenated-name", + "dotted.name", + "channel-has-version-1.0.1", + } + for _, validChannel := range validChannels { + op := operator(operatorsv1alpha1.OperatorSpec{ + PackageName: "package", + Channel: validChannel, + }) + err := cl.Create(ctx, op) + Expect(err).NotTo(HaveOccurred(), "unexpected error creating valid channel '%q': %w", validChannel, err) + err = cl.Delete(ctx, op) + Expect(err).NotTo(HaveOccurred(), "unexpected error deleting valid channel '%q': %w", validChannel, err) + } + }) + It("should fail if an invalid channel name length", func() { + err := cl.Create(ctx, operator(operatorsv1alpha1.OperatorSpec{ + PackageName: "package", + Channel: "longname01234567890123456789012345678901234567890", + })) + Expect(err).To(HaveOccurred(), "expected error for invalid channel length") + Expect(err.Error()).To(ContainSubstring("spec.channel: Too long: may not be longer than 48")) + }) }) diff --git a/controllers/operator_controller_test.go b/controllers/operator_controller_test.go index 822b50d2b..5e7b27c15 100644 --- a/controllers/operator_controller_test.go +++ b/controllers/operator_controller_test.go @@ -85,7 +85,7 @@ var _ = Describe("Operator Controller Test", func() { var pkgName string BeforeEach(func() { By("initializing cluster state") - pkgName = fmt.Sprintf("exists-%s", rand.String(6)) + pkgName = "prometheus" operator = &operatorsv1alpha1.Operator{ ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, Spec: operatorsv1alpha1.OperatorSpec{ @@ -594,6 +594,205 @@ var _ = Describe("Operator Controller Test", func() { }) }) + When("the operator specifies a channel with version that exist", func() { + var pkgName string + var pkgVer string + var pkgChan string + BeforeEach(func() { + By("initializing cluster state") + pkgName = "prometheus" + pkgVer = "0.47.0" + pkgChan = "beta" + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{ + PackageName: pkgName, + Version: pkgVer, + Channel: pkgChan, + }, + } + err := cl.Create(ctx, operator) + Expect(err).NotTo(HaveOccurred()) + }) + It("sets resolution success status", func() { + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).NotTo(HaveOccurred()) + + By("fetching updated operator after reconcile") + Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) + + By("checking the expected conditions") + cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionUnknown)) + Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown)) + Expect(cond.Message).To(ContainSubstring("waiting for BundleDeployment")) + + By("fetching the bundled deployment") + bd := &rukpakv1alpha1.BundleDeployment{} + Expect(cl.Get(ctx, types.NamespacedName{Name: opKey.Name}, bd)).NotTo(HaveOccurred()) + Expect(bd.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-plain")) + Expect(bd.Spec.Template.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-registry")) + Expect(bd.Spec.Template.Spec.Source.Type).To(Equal(rukpakv1alpha1.SourceTypeImage)) + Expect(bd.Spec.Template.Spec.Source.Image).NotTo(BeNil()) + Expect(bd.Spec.Template.Spec.Source.Image.Ref).To(Equal("quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed")) + }) + }) + When("the operator specifies a package that exists within a channel but no version specified", func() { + var pkgName string + var pkgVer string + var pkgChan string + BeforeEach(func() { + By("initializing cluster state") + pkgName = "prometheus" + pkgChan = "beta" + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{ + PackageName: pkgName, + Version: pkgVer, + Channel: pkgChan, + }, + } + err := cl.Create(ctx, operator) + Expect(err).NotTo(HaveOccurred()) + }) + It("sets resolution success status", func() { + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).NotTo(HaveOccurred()) + + By("fetching updated operator after reconcile") + Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) + + By("checking the expected conditions") + cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionUnknown)) + Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonInstallationStatusUnknown)) + Expect(cond.Message).To(ContainSubstring("waiting for BundleDeployment")) + + By("fetching the bundled deployment") + bd := &rukpakv1alpha1.BundleDeployment{} + Expect(cl.Get(ctx, types.NamespacedName{Name: opKey.Name}, bd)).NotTo(HaveOccurred()) + Expect(bd.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-plain")) + Expect(bd.Spec.Template.Spec.ProvisionerClassName).To(Equal("core-rukpak-io-registry")) + Expect(bd.Spec.Template.Spec.Source.Type).To(Equal(rukpakv1alpha1.SourceTypeImage)) + Expect(bd.Spec.Template.Spec.Source.Image).NotTo(BeNil()) + Expect(bd.Spec.Template.Spec.Source.Image.Ref).To(Equal("quay.io/operatorhubio/prometheus@sha256:5b04c49d8d3eff6a338b56ec90bdf491d501fe301c9cdfb740e5bff6769a21ed")) + }) + }) + When("the operator specifies a package version in a channel that does not exist", func() { + var pkgName string + var pkgVer string + var pkgChan string + BeforeEach(func() { + By("initializing cluster state") + pkgName = "prometheus" + pkgVer = "0.47.0" + pkgChan = "alpha" + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{ + PackageName: pkgName, + Version: pkgVer, + Channel: pkgChan, + }, + } + err := cl.Create(ctx, operator) + Expect(err).NotTo(HaveOccurred()) + }) + It("sets resolution failure status", func() { + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).To(MatchError(fmt.Sprintf("package '%s' at version '%s' in channel '%s' not found", pkgName, pkgVer, pkgChan))) + + By("fetching updated operator after reconcile") + Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) + + By("checking the expected conditions") + cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed)) + Expect(cond.Message).To(Equal(fmt.Sprintf("package '%s' at version '%s' in channel '%s' not found", pkgName, pkgVer, pkgChan))) + }) + }) + When("the operator specifies a package in a channel that does not exist", func() { + var pkgName string + var pkgChan string + BeforeEach(func() { + By("initializing cluster state") + pkgName = "prometheus" + pkgChan = "alpha" + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{ + PackageName: pkgName, + Channel: pkgChan, + }, + } + err := cl.Create(ctx, operator) + Expect(err).NotTo(HaveOccurred()) + }) + It("sets resolution failure status", func() { + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).To(MatchError(fmt.Sprintf("package '%s' in channel '%s' not found", pkgName, pkgChan))) + + By("fetching updated operator after reconcile") + Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) + + By("checking the expected conditions") + cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed)) + Expect(cond.Message).To(Equal(fmt.Sprintf("package '%s' in channel '%s' not found", pkgName, pkgChan))) + }) + }) + When("the operator specifies a package version that does not exist in the channel", func() { + var pkgName string + var pkgVer string + var pkgChan string + BeforeEach(func() { + By("initializing cluster state") + pkgName = "prometheus" + pkgVer = "0.57.0" + pkgChan = "beta" + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{ + PackageName: pkgName, + Version: pkgVer, + Channel: pkgChan, + }, + } + err := cl.Create(ctx, operator) + Expect(err).NotTo(HaveOccurred()) + }) + It("sets resolution failure status", func() { + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).To(MatchError(fmt.Sprintf("package '%s' at version '%s' in channel '%s' not found", pkgName, pkgVer, pkgChan))) + + By("fetching updated operator after reconcile") + Expect(cl.Get(ctx, opKey, operator)).NotTo(HaveOccurred()) + + By("checking the expected conditions") + cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorsv1alpha1.TypeReady) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(operatorsv1alpha1.ReasonResolutionFailed)) + Expect(cond.Message).To(Equal(fmt.Sprintf("package '%s' at version '%s' in channel '%s' not found", pkgName, pkgVer, pkgChan))) + }) + }) AfterEach(func() { verifyInvariants(ctx, operator) diff --git a/internal/resolution/variable_sources/olm/olm.go b/internal/resolution/variable_sources/olm/olm.go index faf8b5c7d..97fae6f80 100644 --- a/internal/resolution/variable_sources/olm/olm.go +++ b/internal/resolution/variable_sources/olm/olm.go @@ -46,5 +46,8 @@ func (o *OLMVariableSource) requiredPackageFromOperator(operator *operatorsv1alp if operator.Spec.Version != "" { opts = append(opts, required_package.InVersionRange(operator.Spec.Version)) } + if operator.Spec.Channel != "" { + opts = append(opts, required_package.InChannel(operator.Spec.Channel)) + } return required_package.NewRequiredPackage(operator.Spec.PackageName, opts...) } diff --git a/internal/resolution/variable_sources/required_package/required_package.go b/internal/resolution/variable_sources/required_package/required_package.go index fd577a5b3..94bb13806 100644 --- a/internal/resolution/variable_sources/required_package/required_package.go +++ b/internal/resolution/variable_sources/required_package/required_package.go @@ -54,9 +54,20 @@ func InVersionRange(versionRange string) RequiredPackageOption { } } +func InChannel(channelName string) RequiredPackageOption { + return func(r *RequiredPackageVariableSource) error { + if channelName != "" { + r.channelName = channelName + r.predicates = append(r.predicates, predicates.InChannel(channelName)) + } + return nil + } +} + type RequiredPackageVariableSource struct { packageName string versionRange string + channelName string predicates []input.Predicate } @@ -99,8 +110,14 @@ func (r *RequiredPackageVariableSource) notFoundError() error { // context: we originally wanted to support version ranges and take the highest version that satisfies the range // during the upstream call on the 2023-04-11 we decided to pin the version instead. But, we'll keep version range // support under the covers in case we decide to pivot back. + if r.versionRange != "" && r.channelName != "" { + return fmt.Errorf("package '%s' at version '%s' in channel '%s' not found", r.packageName, r.versionRange, r.channelName) + } if r.versionRange != "" { return fmt.Errorf("package '%s' at version '%s' not found", r.packageName, r.versionRange) } + if r.channelName != "" { + return fmt.Errorf("package '%s' in channel '%s' not found", r.packageName, r.channelName) + } return fmt.Errorf("package '%s' not found", r.packageName) }