diff --git a/api/v1alpha1/operator_types.go b/api/v1alpha1/operator_types.go index 1234e8e68..ddd11ea76 100644 --- a/api/v1alpha1/operator_types.go +++ b/api/v1alpha1/operator_types.go @@ -27,6 +27,16 @@ type OperatorSpec struct { //+kubebuilder:validation:MaxLength:=48 //+kubebuilder:validation:Pattern:=^[a-z0-9]+(-[a-z0-9]+)*$ PackageName string `json:"packageName"` + + //+kubebuilder:validation:MaxLength:=64 + //+kubebuilder:validation:Pattern=^(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-]+)*))?$ + //+kubebuilder:Optional + // Version is an optional semver constraint on the package version. If not specified, the latest version available of the package will be installed. + // If specified, the specific version of the package will be installed so long as it is available in any of the content sources available. + // Examples: 1.2.3, 1.0.0-alpha, 1.0.0-rc.1 + // + // For more information on semver, please see https://semver.org/ + Version string `json:"version,omitempty"` } const ( @@ -38,6 +48,7 @@ const ( ReasonBundleLookupFailed = "BundleLookupFailed" ReasonInstallationFailed = "InstallationFailed" ReasonInstallationStatusUnknown = "InstallationStatusUnknown" + ReasonInvalidSpec = "InvalidSpec" ) func init() { @@ -52,6 +63,7 @@ func init() { ReasonBundleLookupFailed, ReasonInstallationFailed, ReasonInstallationStatusUnknown, + ReasonInvalidSpec, ) } diff --git a/config/crd/bases/operators.operatorframework.io_operators.yaml b/config/crd/bases/operators.operatorframework.io_operators.yaml index 894b7699c..45a177cc8 100644 --- a/config/crd/bases/operators.operatorframework.io_operators.yaml +++ b/config/crd/bases/operators.operatorframework.io_operators.yaml @@ -39,6 +39,16 @@ spec: maxLength: 48 pattern: ^[a-z0-9]+(-[a-z0-9]+)*$ type: string + version: + description: "Version is an optional semver constraint on the package + version. If not specified, the latest version available of the package + will be installed. If specified, the specific version of the package + will be installed so long as it is available in any of the content + sources available. Examples: 1.2.3, 1.0.0-alpha, 1.0.0-rc.1 \n For + more information on semver, please see https://semver.org/" + maxLength: 64 + pattern: ^(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-]+)*))?$ + type: string required: - packageName type: object diff --git a/controllers/admission_test.go b/controllers/admission_test.go new file mode 100644 index 000000000..9e3414624 --- /dev/null +++ b/controllers/admission_test.go @@ -0,0 +1,75 @@ +package controllers_test + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" +) + +func operator(spec operatorsv1alpha1.OperatorSpec) *operatorsv1alpha1.Operator { + return &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-operator", + }, + Spec: spec, + } +} + +var _ = Describe("Operator Spec Validations", func() { + var ( + ctx context.Context + cancel context.CancelFunc + ) + BeforeEach(func() { + ctx, cancel = context.WithCancel(context.Background()) + }) + AfterEach(func() { + cancel() + }) + It("should fail if the spec is empty", func() { + err := cl.Create(ctx, operator(operatorsv1alpha1.OperatorSpec{})) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("spec.packageName in body should match '^[a-z0-9]+(-[a-z0-9]+)*$'")) + }) + It("should fail if package name length is greater than 48 characters", func() { + err := cl.Create(ctx, operator(operatorsv1alpha1.OperatorSpec{ + PackageName: "this-is-a-really-long-package-name-that-is-greater-than-48-characters", + })) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Too long: may not be longer than 48")) + }) + It("should fail if version is valid semver but length is greater than 64 characters", func() { + err := cl.Create(ctx, operator(operatorsv1alpha1.OperatorSpec{ + PackageName: "package", + Version: "1234567890.1234567890.12345678901234567890123456789012345678901234", + })) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("Too long: may not be longer than 64")) + }) + It("should fail if an invalid semver is given", func() { + invalidSemvers := []string{ + "1.2.3.4", + "1.02.3", + "1.2.03", + "1.2.3-beta!", + "1.2.3.alpha", + "1..2.3", + "1.2.3-pre+bad_metadata", + "1.2.-3", + ".1.2.3", + } + for _, invalidSemver := range invalidSemvers { + err := cl.Create(ctx, operator(operatorsv1alpha1.OperatorSpec{ + PackageName: "package", + Version: invalidSemver, + })) + + Expect(err).To(HaveOccurred(), "expected error for invalid semver %q", invalidSemver) + 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-]+)*))?$'")) + } + }) +}) diff --git a/controllers/operator_controller.go b/controllers/operator_controller.go index 73e554c69..9c12e89e9 100644 --- a/controllers/operator_controller.go +++ b/controllers/operator_controller.go @@ -34,6 +34,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" + "github.com/operator-framework/operator-controller/controllers/validators" + operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/resolution" "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundles_and_dependencies" @@ -108,6 +110,18 @@ func checkForUnexpectedFieldChange(a, b operatorsv1alpha1.Operator) bool { // Helper function to do the actual reconcile func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha1.Operator) (ctrl.Result, error) { + // validate spec + if err := validators.ValidateOperatorSpec(op); err != nil { + apimeta.SetStatusCondition(&op.Status.Conditions, metav1.Condition{ + Type: operatorsv1alpha1.TypeReady, + Status: metav1.ConditionFalse, + Reason: operatorsv1alpha1.ReasonInvalidSpec, + Message: err.Error(), + ObservedGeneration: op.GetGeneration(), + }) + return ctrl.Result{}, nil + } + // run resolution solution, err := r.Resolver.Resolve(ctx) if err != nil { diff --git a/controllers/operator_controller_test.go b/controllers/operator_controller_test.go index a8efb3a2a..822b50d2b 100644 --- a/controllers/operator_controller_test.go +++ b/controllers/operator_controller_test.go @@ -16,6 +16,7 @@ import ( "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/controllers" @@ -80,6 +81,38 @@ var _ = Describe("Operator Controller Test", func() { Expect(cond.Message).To(Equal(fmt.Sprintf("package '%s' not found", pkgName))) }) }) + When("the operator specifies a version that does not exist", func() { + var pkgName string + BeforeEach(func() { + By("initializing cluster state") + pkgName = fmt.Sprintf("exists-%s", rand.String(6)) + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{ + PackageName: pkgName, + Version: "0.50.0", // this version of the package does not exist + }, + } + 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 '0.50.0' not found", pkgName))) + + 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 '0.50.0' not found", pkgName))) + }) + }) When("the operator specifies a valid available package", func() { const pkgName = "prometheus" BeforeEach(func() { @@ -568,6 +601,54 @@ var _ = Describe("Operator Controller Test", func() { Expect(err).To(Not(HaveOccurred())) }) }) + When("an invalid semver is provided that bypasses the regex validation", func() { + var ( + operator *operatorsv1alpha1.Operator + opKey types.NamespacedName + pkgName string + fakeClient client.Client + ) + BeforeEach(func() { + opKey = types.NamespacedName{Name: fmt.Sprintf("operator-validation-test-%s", rand.String(8))} + + By("injecting creating a client with the bad operator CR") + pkgName = fmt.Sprintf("exists-%s", rand.String(6)) + operator = &operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{Name: opKey.Name}, + Spec: operatorsv1alpha1.OperatorSpec{ + PackageName: pkgName, + Version: "1.2.3-123abc_def", // bad semver that matches the regex on the CR validation + }, + } + + // this bypasses client/server-side CR validation and allows us to test the reconciler's validation + fakeClient = fake.NewClientBuilder().WithScheme(sch).WithObjects(operator).Build() + + By("changing the reconciler client to the fake client") + reconciler.Client = fakeClient + }) + AfterEach(func() { + By("changing the reconciler client back to the real client") + reconciler.Client = cl + }) + + It("should add an invalid spec condition and *not* re-enqueue for reconciliation", func() { + By("running reconcile") + res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: opKey}) + Expect(res).To(Equal(ctrl.Result{})) + Expect(err).ToNot(HaveOccurred()) + + By("fetching updated operator after reconcile") + Expect(fakeClient.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.ReasonInvalidSpec)) + Expect(cond.Message).To(Equal("invalid .spec.version: Invalid character(s) found in prerelease \"123abc_def\"")) + }) + }) }) func verifyInvariants(ctx context.Context, op *operatorsv1alpha1.Operator) { diff --git a/controllers/validators/validators.go b/controllers/validators/validators.go new file mode 100644 index 000000000..93f777fe4 --- /dev/null +++ b/controllers/validators/validators.go @@ -0,0 +1,43 @@ +package validators + +import ( + "fmt" + + "github.com/blang/semver/v4" + + operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" +) + +type operatorCRValidatorFunc func(operator *operatorsv1alpha1.Operator) error + +// validateSemver validates that the operator's version is a valid SemVer. +// this validation should already be happening at the CRD level. But, it depends +// on a regex that could possibly fail to validate a valid SemVer. This is added as an +// extra measure to ensure a valid spec before the CR is processed for resolution +func validateSemver(operator *operatorsv1alpha1.Operator) error { + if operator.Spec.Version == "" { + return nil + } + if _, err := semver.Parse(operator.Spec.Version); err != nil { + return fmt.Errorf("invalid .spec.version: %w", err) + } + return nil +} + +// ValidateOperatorSpec validates the operator spec, e.g. ensuring that .spec.version, if provided, is a valid SemVer +func ValidateOperatorSpec(operator *operatorsv1alpha1.Operator) error { + validators := []operatorCRValidatorFunc{ + validateSemver, + } + + // TODO: currently we only have a single validator, but more will likely be added in the future + // we need to make a decision on whether we want to run all validators or stop at the first error. If the the former, + // we should consider how to present this to the user in a way that is easy to understand and fix. + // this issue is tracked here: https://github.com/operator-framework/operator-controller/issues/167 + for _, validator := range validators { + if err := validator(operator); err != nil { + return err + } + } + return nil +} diff --git a/controllers/validators/validators_suite_test.go b/controllers/validators/validators_suite_test.go new file mode 100644 index 000000000..21c625c18 --- /dev/null +++ b/controllers/validators/validators_suite_test.go @@ -0,0 +1,13 @@ +package validators + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestValidators(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Validators Suite") +} diff --git a/controllers/validators/validators_test.go b/controllers/validators/validators_test.go new file mode 100644 index 000000000..f00e2e789 --- /dev/null +++ b/controllers/validators/validators_test.go @@ -0,0 +1,63 @@ +package validators_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/operator-framework/operator-controller/api/v1alpha1" + "github.com/operator-framework/operator-controller/controllers/validators" +) + +var _ = Describe("Validators", func() { + Describe("ValidateOperatorSpec", func() { + It("should not return an error for valid SemVer", func() { + operator := &v1alpha1.Operator{ + Spec: v1alpha1.OperatorSpec{ + Version: "1.2.3", + }, + } + err := validators.ValidateOperatorSpec(operator) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should return an error for invalid SemVer", func() { + operator := &v1alpha1.Operator{ + Spec: v1alpha1.OperatorSpec{ + Version: "invalid-semver", + }, + } + err := validators.ValidateOperatorSpec(operator) + Expect(err).To(HaveOccurred()) + }) + + It("should not return an error for empty SemVer", func() { + operator := &v1alpha1.Operator{ + Spec: v1alpha1.OperatorSpec{ + Version: "", + }, + } + err := validators.ValidateOperatorSpec(operator) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should not return an error for valid SemVer with pre-release and metadata", func() { + operator := &v1alpha1.Operator{ + Spec: v1alpha1.OperatorSpec{ + Version: "1.2.3-alpha.1+metadata", + }, + } + err := validators.ValidateOperatorSpec(operator) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should not return an error for valid SemVer with pre-release", func() { + operator := &v1alpha1.Operator{ + Spec: v1alpha1.OperatorSpec{ + Version: "1.2.3-alpha-beta", + }, + } + err := validators.ValidateOperatorSpec(operator) + Expect(err).NotTo(HaveOccurred()) + }) + }) +}) diff --git a/go.mod b/go.mod index b36c7b385..045bd045e 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/operator-framework/deppy v0.0.0-20230125110717-dc02e928470f github.com/operator-framework/operator-registry v1.26.2 github.com/operator-framework/rukpak v0.11.0 + go.uber.org/zap v1.21.0 golang.org/x/net v0.0.0-20220722155237-a158d28d115b google.golang.org/grpc v1.47.0 k8s.io/api v0.25.0 @@ -70,7 +71,6 @@ require ( github.com/spf13/pflag v1.0.5 // indirect go.uber.org/atomic v1.7.0 // indirect go.uber.org/multierr v1.6.0 // indirect - go.uber.org/zap v1.21.0 // indirect golang.org/x/crypto v0.0.0-20220408190544-5352b0902921 // indirect golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8 // indirect golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f // indirect diff --git a/internal/resolution/resolver.go b/internal/resolution/resolver.go index f0b2dd571..1ffd1a221 100644 --- a/internal/resolution/resolver.go +++ b/internal/resolution/resolver.go @@ -2,7 +2,6 @@ package resolution import ( "context" - "fmt" "github.com/operator-framework/deppy/pkg/deppy/input" "github.com/operator-framework/deppy/pkg/deppy/solver" @@ -26,11 +25,15 @@ func NewOperatorResolver(client client.Client, entitySource input.EntitySource) } func (o *OperatorResolver) Resolve(ctx context.Context) (*solver.Solution, error) { - packageNames, err := o.getPackageNames(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get package names for resolution: %w", err) + operatorList := v1alpha1.OperatorList{} + if err := o.client.List(ctx, &operatorList); err != nil { + return nil, err } - olmVariableSource := olm.NewOLMVariableSource(packageNames...) + if len(operatorList.Items) == 0 { + return &solver.Solution{}, nil + } + + olmVariableSource := olm.NewOLMVariableSource(operatorList.Items...) deppySolver, err := solver.NewDeppySolver(o.entitySource, olmVariableSource) if err != nil { return nil, err @@ -42,15 +45,3 @@ func (o *OperatorResolver) Resolve(ctx context.Context) (*solver.Solution, error } return solution, nil } - -func (o *OperatorResolver) getPackageNames(ctx context.Context) ([]string, error) { - operatorList := v1alpha1.OperatorList{} - if err := o.client.List(ctx, &operatorList); err != nil { - return nil, err - } - var packageNames []string - for _, operator := range operatorList.Items { - packageNames = append(packageNames, operator.Spec.PackageName) - } - return packageNames, nil -} diff --git a/internal/resolution/variable_sources/olm/olm.go b/internal/resolution/variable_sources/olm/olm.go index 87502f802..faf8b5c7d 100644 --- a/internal/resolution/variable_sources/olm/olm.go +++ b/internal/resolution/variable_sources/olm/olm.go @@ -6,6 +6,7 @@ import ( "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" + operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundles_and_dependencies" "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/crd_constraints" "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package" @@ -14,12 +15,12 @@ import ( var _ input.VariableSource = &OLMVariableSource{} type OLMVariableSource struct { - packageNames []string + operators []operatorsv1alpha1.Operator } -func NewOLMVariableSource(packageNames ...string) *OLMVariableSource { +func NewOLMVariableSource(operators ...operatorsv1alpha1.Operator) *OLMVariableSource { return &OLMVariableSource{ - packageNames: packageNames, + operators: operators, } } @@ -27,11 +28,23 @@ func (o *OLMVariableSource) GetVariables(ctx context.Context, entitySource input var inputVariableSources []input.VariableSource // build required package variable sources - for _, packageName := range o.packageNames { - inputVariableSources = append(inputVariableSources, required_package.NewRequiredPackage(packageName)) + for _, operator := range o.operators { + rps, err := o.requiredPackageFromOperator(&operator) + if err != nil { + return nil, err + } + inputVariableSources = append(inputVariableSources, rps) } // build variable source pipeline variableSource := crd_constraints.NewCRDUniquenessConstraintsVariableSource(bundles_and_dependencies.NewBundlesAndDepsVariableSource(inputVariableSources...)) return variableSource.GetVariables(ctx, entitySource) } + +func (o *OLMVariableSource) requiredPackageFromOperator(operator *operatorsv1alpha1.Operator) (*required_package.RequiredPackageVariableSource, error) { + var opts []required_package.RequiredPackageOption + if operator.Spec.Version != "" { + opts = append(opts, required_package.InVersionRange(operator.Spec.Version)) + } + return required_package.NewRequiredPackage(operator.Spec.PackageName, opts...) +} diff --git a/internal/resolution/variable_sources/olm/olm_test.go b/internal/resolution/variable_sources/olm/olm_test.go index 814666a9a..e3010fb4b 100644 --- a/internal/resolution/variable_sources/olm/olm_test.go +++ b/internal/resolution/variable_sources/olm/olm_test.go @@ -11,10 +11,12 @@ import ( . "github.com/onsi/gomega" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/input" + operatorsv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1" "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/bundles_and_dependencies" "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/crd_constraints" "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/olm" "github.com/operator-framework/operator-controller/internal/resolution/variable_sources/required_package" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestGlobalConstraints(t *testing.T) { @@ -48,6 +50,32 @@ func entityFromCache(name string) *input.Entity { return &entity } +type opOption func(*operatorsv1alpha1.Operator) error + +func withVersionRange(versionRange string) opOption { + return func(op *operatorsv1alpha1.Operator) error { + op.Spec.Version = versionRange + return nil + } +} + +func operator(name string, opts ...opOption) operatorsv1alpha1.Operator { + op := operatorsv1alpha1.Operator{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: operatorsv1alpha1.OperatorSpec{ + PackageName: name, + }, + } + for _, opt := range opts { + if err := opt(&op); err != nil { + Fail(err.Error()) + } + } + return op +} + var _ = Describe("OLMVariableSource", func() { var testEntitySource input.EntitySource @@ -56,7 +84,7 @@ var _ = Describe("OLMVariableSource", func() { }) It("should produce RequiredPackage variables", func() { - olmVariableSource := olm.NewOLMVariableSource("prometheus", "packageA") + olmVariableSource := olm.NewOLMVariableSource(operator("prometheus"), operator("packageA")) variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) @@ -69,7 +97,7 @@ var _ = Describe("OLMVariableSource", func() { }) It("should produce BundleVariables variables", func() { - olmVariableSource := olm.NewOLMVariableSource("prometheus", "packageA") + olmVariableSource := olm.NewOLMVariableSource(operator("prometheus"), operator("packageA")) variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) @@ -88,8 +116,29 @@ var _ = Describe("OLMVariableSource", func() { }))) }) + It("should produce version filtered BundleVariables variables", func() { + olmVariableSource := olm.NewOLMVariableSource(operator("prometheus", withVersionRange(">0.40.0")), operator("packageA")) + variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) + Expect(err).ToNot(HaveOccurred()) + + bundleVariables := filterVariables[*bundles_and_dependencies.BundleVariable](variables) + Expect(bundleVariables).To(HaveLen(2)) + Expect(bundleVariables).To(WithTransform(func(bvars []*bundles_and_dependencies.BundleVariable) []*input.Entity { + var out []*input.Entity + for _, variable := range bvars { + out = append(out, variable.BundleEntity().Entity) + } + return out + }, Equal([]*input.Entity{ + entityFromCache("operatorhub/prometheus/0.47.0"), + // filtered out + // entityFromCache("operatorhub/prometheus/0.37.0"), + entityFromCache("operatorhub/packageA/2.0.0"), + }))) + }) + It("should produce GlobalConstraints variables", func() { - olmVariableSource := olm.NewOLMVariableSource("prometheus", "packageA") + olmVariableSource := olm.NewOLMVariableSource(operator("prometheus"), operator("packageA")) variables, err := olmVariableSource.GetVariables(context.Background(), testEntitySource) Expect(err).ToNot(HaveOccurred()) @@ -117,7 +166,7 @@ var _ = Describe("OLMVariableSource", func() { }) It("should return an errors when they occur", func() { - olmVariableSource := olm.NewOLMVariableSource("prometheus", "packageA") + olmVariableSource := olm.NewOLMVariableSource(operator("prometheus"), operator("packageA")) _, err := olmVariableSource.GetVariables(context.Background(), FailEntitySource{}) Expect(err).To(HaveOccurred()) }) diff --git a/internal/resolution/variable_sources/required_package/required_package.go b/internal/resolution/variable_sources/required_package/required_package.go index 36dbb4b5c..fd577a5b3 100644 --- a/internal/resolution/variable_sources/required_package/required_package.go +++ b/internal/resolution/variable_sources/required_package/required_package.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/blang/semver/v4" "github.com/operator-framework/deppy/pkg/deppy" "github.com/operator-framework/deppy/pkg/deppy/constraint" "github.com/operator-framework/deppy/pkg/deppy/input" @@ -36,23 +37,52 @@ func NewRequiredPackageVariable(packageName string, bundleEntities []*olmentity. var _ input.VariableSource = &RequiredPackageVariableSource{} +type RequiredPackageOption func(*RequiredPackageVariableSource) error + +func InVersionRange(versionRange string) RequiredPackageOption { + return func(r *RequiredPackageVariableSource) error { + if versionRange != "" { + if vr, err := semver.ParseRange(versionRange); err == nil { + r.versionRange = versionRange + r.predicates = append(r.predicates, predicates.InSemverRange(vr)) + return nil + } else { + return fmt.Errorf("invalid version range '%s': %v", versionRange, err) + } + } + return nil + } +} + type RequiredPackageVariableSource struct { - packageName string + packageName string + versionRange string + predicates []input.Predicate } -func NewRequiredPackage(packageName string) *RequiredPackageVariableSource { - return &RequiredPackageVariableSource{ +func NewRequiredPackage(packageName string, options ...RequiredPackageOption) (*RequiredPackageVariableSource, error) { + if packageName == "" { + return nil, fmt.Errorf("package name must not be empty") + } + r := &RequiredPackageVariableSource{ packageName: packageName, + predicates: []input.Predicate{predicates.WithPackageName(packageName)}, } + for _, option := range options { + if err := option(r); err != nil { + return nil, err + } + } + return r, nil } func (r *RequiredPackageVariableSource) GetVariables(ctx context.Context, entitySource input.EntitySource) ([]deppy.Variable, error) { - resultSet, err := entitySource.Filter(ctx, predicates.WithPackageName(r.packageName)) + resultSet, err := entitySource.Filter(ctx, input.And(r.predicates...)) if err != nil { return nil, err } if len(resultSet) == 0 { - return nil, fmt.Errorf("package '%s' not found", r.packageName) + return nil, r.notFoundError() } resultSet = resultSet.Sort(sort.ByChannelAndVersion) var bundleEntities []*olmentity.BundleEntity @@ -63,3 +93,14 @@ func (r *RequiredPackageVariableSource) GetVariables(ctx context.Context, entity NewRequiredPackageVariable(r.packageName, bundleEntities), }, nil } + +func (r *RequiredPackageVariableSource) notFoundError() error { + // TODO: update this error message when/if we decide to support version ranges as opposed to fixing the version + // 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 != "" { + return fmt.Errorf("package '%s' at version '%s' not found", r.packageName, r.versionRange) + } + return fmt.Errorf("package '%s' not found", r.packageName) +} diff --git a/internal/resolution/variable_sources/required_package/required_package_test.go b/internal/resolution/variable_sources/required_package/required_package_test.go index 3834029d6..6c9b4babb 100644 --- a/internal/resolution/variable_sources/required_package/required_package_test.go +++ b/internal/resolution/variable_sources/required_package/required_package_test.go @@ -68,8 +68,10 @@ var _ = Describe("RequiredPackageVariableSource", func() { ) BeforeEach(func() { + var err error packageName = "test-package" - rpvs = required_package.NewRequiredPackage(packageName) + rpvs, err = required_package.NewRequiredPackage(packageName) + Expect(err).NotTo(HaveOccurred()) mockEntitySource = input.NewCacheQuerier(map[deppy.Identifier]input.Entity{ "bundle-1": *input.NewEntity("bundle-1", map[string]string{ property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, @@ -104,7 +106,7 @@ var _ = Describe("RequiredPackageVariableSource", func() { Expect(ok).To(BeTrue()) Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) - // ensure bundle entities are in version order + // ensure bundle entities are in version order (high to low) Expect(reqPackageVar.BundleEntities()).To(Equal([]*olmentity.BundleEntity{ olmentity.NewBundleEntity(input.NewEntity("bundle-2", map[string]string{ property.TypePackage: `{"packageName": "test-package", "version": "3.0.0"}`, @@ -120,6 +122,33 @@ var _ = Describe("RequiredPackageVariableSource", func() { })) }) + It("should filter by version range", func() { + // recreate source with version range option + var err error + rpvs, err = required_package.NewRequiredPackage(packageName, required_package.InVersionRange(">=1.0.0 !2.0.0 <3.0.0")) + Expect(err).NotTo(HaveOccurred()) + + variables, err := rpvs.GetVariables(context.TODO(), mockEntitySource) + Expect(err).NotTo(HaveOccurred()) + Expect(len(variables)).To(Equal(1)) + reqPackageVar, ok := variables[0].(*required_package.RequiredPackageVariable) + Expect(ok).To(BeTrue()) + Expect(reqPackageVar.Identifier()).To(Equal(deppy.IdentifierFromString(fmt.Sprintf("required package %s", packageName)))) + + // ensure bundle entities are in version order (high to low) + Expect(reqPackageVar.BundleEntities()).To(Equal([]*olmentity.BundleEntity{ + olmentity.NewBundleEntity(input.NewEntity("bundle-1", map[string]string{ + property.TypePackage: `{"packageName": "test-package", "version": "1.0.0"}`, + property.TypeChannel: `{"channelName":"stable","priority":0}`, + })), + })) + }) + + It("should fail with bad semver range", func() { + _, err := required_package.NewRequiredPackage(packageName, required_package.InVersionRange("not a valid semver")) + Expect(err).To(HaveOccurred()) + }) + It("should return an error if package not found", func() { mockEntitySource := input.NewCacheQuerier(map[deppy.Identifier]input.Entity{}) _, err := rpvs.GetVariables(context.TODO(), mockEntitySource) diff --git a/main.go b/main.go index df2f804d0..0c26d4b35 100644 --- a/main.go +++ b/main.go @@ -22,6 +22,7 @@ import ( olmv0v1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1" rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1" + "go.uber.org/zap/zapcore" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -65,7 +66,7 @@ func main() { opts.BindFlags(flag.CommandLine) flag.Parse() - ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts), zap.StacktraceLevel(zapcore.DPanicLevel))) mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme,