From b01a375bb7fdfbc46706c65ba9450a4af5c931cc Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Fri, 11 Apr 2025 18:19:41 +0200 Subject: [PATCH 01/11] add webhook bundle validators Signed-off-by: Per Goncalves da Silva --- .../rukpak/render/validators/validator.go | 150 +++++ .../render/validators/validator_test.go | 623 +++++++++++++++++- .../rukpak/util/testing/testing.go | 6 + 3 files changed, 778 insertions(+), 1 deletion(-) diff --git a/internal/operator-controller/rukpak/render/validators/validator.go b/internal/operator-controller/rukpak/render/validators/validator.go index d2ed950e5..ee77292a3 100644 --- a/internal/operator-controller/rukpak/render/validators/validator.go +++ b/internal/operator-controller/rukpak/render/validators/validator.go @@ -1,12 +1,17 @@ package validators import ( + "cmp" "errors" "fmt" + "maps" "slices" + "strings" "k8s.io/apimachinery/pkg/util/sets" + "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" ) @@ -19,6 +24,10 @@ var RegistryV1BundleValidator = render.BundleValidator{ CheckCRDResourceUniqueness, CheckOwnedCRDExistence, CheckPackageNameNotEmpty, + CheckWebhookDeploymentReferentialIntegrity, + CheckWebhookNameUniqueness, + CheckConversionWebhookCRDReferenceUniqueness, + CheckConversionWebhooksReferenceOwnedCRDs, } // CheckDeploymentSpecUniqueness checks that each strategy deployment spec in the csv has a unique name. @@ -86,3 +95,144 @@ func CheckPackageNameNotEmpty(rv1 *render.RegistryV1) []error { } return nil } + +// CheckWebhookSupport checks that if the bundle cluster service version declares webhook definitions +// that it is a singleton operator, i.e. that it only supports AllNamespaces mode. This keeps parity +// with OLMv0 behavior for conversion webhooks, +// https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/webhook.go#L193 +// Since OLMv1 considers APIs to be cluster-scoped, we initially extend this constraint to validating and mutating webhooks. +// While this might restrict the number of supported bundles, we can tackle the issue of relaxing this constraint in turn +// after getting the webhook support working. +func CheckWebhookSupport(rv1 *render.RegistryV1) []error { + if len(rv1.CSV.Spec.WebhookDefinitions) > 0 { + supportedInstallModes := sets.Set[v1alpha1.InstallModeType]{} + for _, mode := range rv1.CSV.Spec.InstallModes { + supportedInstallModes.Insert(mode.Type) + } + if len(supportedInstallModes) != 1 || !supportedInstallModes.Has(v1alpha1.InstallModeTypeAllNamespaces) { + return []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")} + } + } + + return nil +} + +// CheckWebhookDeploymentReferentialIntegrity checks that each webhook definition in the csv +// references an existing strategy deployment spec. Errors are sorted by strategy deployment spec name, +// webhook type, and webhook name. +func CheckWebhookDeploymentReferentialIntegrity(rv1 *render.RegistryV1) []error { + webhooksByDeployment := map[string][]v1alpha1.WebhookDescription{} + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + webhooksByDeployment[wh.DeploymentName] = append(webhooksByDeployment[wh.DeploymentName], wh) + } + + for _, depl := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { + delete(webhooksByDeployment, depl.Name) + } + + var errs []error + // Loop through sorted keys to keep error messages ordered by deployment name + for _, deploymentName := range slices.Sorted(maps.Keys(webhooksByDeployment)) { + webhookDefns := webhooksByDeployment[deploymentName] + slices.SortFunc(webhookDefns, func(a, b v1alpha1.WebhookDescription) int { + return cmp.Or(cmp.Compare(a.Type, b.Type), cmp.Compare(a.GenerateName, b.GenerateName)) + }) + for _, webhookDef := range webhookDefns { + errs = append(errs, fmt.Errorf("webhook '%s' of type '%s' references non-existent deployment '%s'", webhookDef.GenerateName, webhookDef.Type, webhookDef.DeploymentName)) + } + } + return errs +} + +// CheckWebhookNameUniqueness checks that each webhook definition of each type (validating, mutating, or conversion) +// has a unique name. Webhooks of different types can have the same name. Errors are sorted by webhook type +// and name. +func CheckWebhookNameUniqueness(rv1 *render.RegistryV1) []error { + webhookNameSetByType := map[v1alpha1.WebhookAdmissionType]sets.Set[string]{} + duplicateWebhooksByType := map[v1alpha1.WebhookAdmissionType]sets.Set[string]{} + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + if _, ok := webhookNameSetByType[wh.Type]; !ok { + webhookNameSetByType[wh.Type] = sets.Set[string]{} + } + if webhookNameSetByType[wh.Type].Has(wh.GenerateName) { + if _, ok := duplicateWebhooksByType[wh.Type]; !ok { + duplicateWebhooksByType[wh.Type] = sets.Set[string]{} + } + duplicateWebhooksByType[wh.Type].Insert(wh.GenerateName) + } + webhookNameSetByType[wh.Type].Insert(wh.GenerateName) + } + + var errs []error + for _, whType := range slices.Sorted(maps.Keys(duplicateWebhooksByType)) { + for _, webhookName := range slices.Sorted(slices.Values(duplicateWebhooksByType[whType].UnsortedList())) { + errs = append(errs, fmt.Errorf("duplicate webhook '%s' of type '%s'", webhookName, whType)) + } + } + return errs +} + +// CheckConversionWebhooksReferenceOwnedCRDs checks defined conversion webhooks reference bundle owned CRDs. +// Errors are sorted by webhook name and CRD name. +func CheckConversionWebhooksReferenceOwnedCRDs(rv1 *render.RegistryV1) []error { + //nolint:prealloc + var conversionWebhooks []v1alpha1.WebhookDescription + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + if wh.Type != v1alpha1.ConversionWebhook { + continue + } + conversionWebhooks = append(conversionWebhooks, wh) + } + + if len(conversionWebhooks) == 0 { + return nil + } + + ownedCRDNames := sets.Set[string]{} + for _, crd := range rv1.CSV.Spec.CustomResourceDefinitions.Owned { + ownedCRDNames.Insert(crd.Name) + } + + slices.SortFunc(conversionWebhooks, func(a, b v1alpha1.WebhookDescription) int { + return cmp.Compare(a.GenerateName, b.GenerateName) + }) + + var errs []error + for _, webhook := range conversionWebhooks { + webhookCRDs := webhook.ConversionCRDs + slices.Sort(webhookCRDs) + for _, crd := range webhookCRDs { + if !ownedCRDNames.Has(crd) { + errs = append(errs, fmt.Errorf("conversion webhook '%s' references custom resource definition '%s' not owned bundle", webhook.GenerateName, crd)) + } + } + } + return errs +} + +// CheckConversionWebhookCRDReferenceUniqueness checks no two (or more) conversion webhooks reference the same CRD. +func CheckConversionWebhookCRDReferenceUniqueness(rv1 *render.RegistryV1) []error { + // collect webhooks by crd + crdToWh := map[string][]string{} + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + if wh.Type != v1alpha1.ConversionWebhook { + continue + } + for _, crd := range wh.ConversionCRDs { + crdToWh[crd] = append(crdToWh[crd], wh.GenerateName) + } + } + + // remove crds with single webhook + maps.DeleteFunc(crdToWh, func(crd string, whs []string) bool { + return len(whs) == 1 + }) + + errs := make([]error, 0, len(crdToWh)) + orderedCRDs := slices.Sorted(maps.Keys(crdToWh)) + for _, crd := range orderedCRDs { + orderedWhs := strings.Join(slices.Sorted(slices.Values(crdToWh[crd])), ",") + errs = append(errs, fmt.Errorf("conversion webhooks [%s] reference same custom resource definition '%s'", orderedWhs, crd)) + } + return errs +} diff --git a/internal/operator-controller/rukpak/render/validators/validator_test.go b/internal/operator-controller/rukpak/render/validators/validator_test.go index 17da3e640..d8a480bf6 100644 --- a/internal/operator-controller/rukpak/render/validators/validator_test.go +++ b/internal/operator-controller/rukpak/render/validators/validator_test.go @@ -13,7 +13,7 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators" - . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" + . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" ) func Test_BundleValidatorHasAllValidationFns(t *testing.T) { @@ -22,6 +22,10 @@ func Test_BundleValidatorHasAllValidationFns(t *testing.T) { validators.CheckCRDResourceUniqueness, validators.CheckOwnedCRDExistence, validators.CheckPackageNameNotEmpty, + validators.CheckWebhookDeploymentReferentialIntegrity, + validators.CheckWebhookNameUniqueness, + validators.CheckConversionWebhookCRDReferenceUniqueness, + validators.CheckConversionWebhooksReferenceOwnedCRDs, } actualValidationFns := validators.RegistryV1BundleValidator @@ -216,3 +220,620 @@ func Test_CheckPackageNameNotEmpty(t *testing.T) { }) } } + +func Test_CheckWebhookSupport(t *testing.T) { + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + expectedErrs []error + }{ + { + name: "accepts bundles with validating webhook definitions when they only support AllNamespaces install mode", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + }, + ), + ), + }, + }, + { + name: "accepts bundles with mutating webhook definitions when they only support AllNamespaces install mode", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + }, + ), + ), + }, + }, + { + name: "accepts bundles with conversion webhook definitions when they only support AllNamespaces install mode", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + }, + ), + ), + }, + }, + { + name: "rejects bundles with validating webhook definitions when they support more modes than AllNamespaces install mode", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + }, + ), + ), + }, + expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")}, + }, + { + name: "accepts bundles with mutating webhook definitions when they support more modes than AllNamespaces install mode", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + }, + ), + ), + }, + expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")}, + }, + { + name: "accepts bundles with conversion webhook definitions when they support more modes than AllNamespaces install mode", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + }, + ), + ), + }, + expectedErrs: []error{errors.New("bundle contains webhook definitions but supported install modes beyond AllNamespaces")}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := validators.CheckWebhookSupport(tc.bundle) + require.Equal(t, tc.expectedErrs, errs) + }) + } +} + +func Test_CheckWebhookDeploymentReferentialIntegrity(t *testing.T) { + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + expectedErrs []error + }{ + { + name: "accepts bundles where webhook definitions reference existing strategy deployment specs", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, + v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-two"}, + ), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-webhook", + DeploymentName: "test-deployment-one", + }, + ), + ), + }, + }, { + name: "rejects bundles with webhook definitions that reference non-existing strategy deployment specs", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, + ), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-webhook", + DeploymentName: "test-deployment-two", + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("webhook 'test-webhook' of type 'ValidatingAdmissionWebhook' references non-existent deployment 'test-deployment-two'"), + }, + }, { + name: "errors are ordered by deployment strategy spec name, webhook type, and webhook name", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, + ), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-val-webhook-c", + DeploymentName: "test-deployment-c", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-mute-webhook-a", + DeploymentName: "test-deployment-a", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-conv-webhook-b", + DeploymentName: "test-deployment-b", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-mute-webhook-c", + DeploymentName: "test-deployment-c", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-conv-webhook-c-b", + DeploymentName: "test-deployment-c", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-conv-webhook-c-a", + DeploymentName: "test-deployment-c", + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("webhook 'test-mute-webhook-a' of type 'MutatingAdmissionWebhook' references non-existent deployment 'test-deployment-a'"), + errors.New("webhook 'test-conv-webhook-b' of type 'ConversionWebhook' references non-existent deployment 'test-deployment-b'"), + errors.New("webhook 'test-conv-webhook-c-a' of type 'ConversionWebhook' references non-existent deployment 'test-deployment-c'"), + errors.New("webhook 'test-conv-webhook-c-b' of type 'ConversionWebhook' references non-existent deployment 'test-deployment-c'"), + errors.New("webhook 'test-mute-webhook-c' of type 'MutatingAdmissionWebhook' references non-existent deployment 'test-deployment-c'"), + errors.New("webhook 'test-val-webhook-c' of type 'ValidatingAdmissionWebhook' references non-existent deployment 'test-deployment-c'"), + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := validators.CheckWebhookDeploymentReferentialIntegrity(tc.bundle) + require.Equal(t, tc.expectedErrs, errs) + }) + } +} + +func Test_CheckWebhookNameUniqueness(t *testing.T) { + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + expectedErrs []error + }{ + { + name: "accepts bundles without webhook definitions", + bundle: &render.RegistryV1{ + CSV: MakeCSV(), + }, + }, { + name: "accepts bundles with unique webhook names", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-webhook-one", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-webhook-two", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook-three", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-webhook-four", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-webhook-five", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook-six", + }, + ), + ), + }, + }, { + name: "accepts bundles with webhooks with the same name but different types", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-webhook", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-webhook", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook", + }, + ), + ), + }, + }, { + name: "rejects bundles with duplicate validating webhook definitions", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-webhook", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-webhook", + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("duplicate webhook 'test-webhook' of type 'ValidatingAdmissionWebhook'"), + }, + }, { + name: "rejects bundles with duplicate mutating webhook definitions", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-webhook", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-webhook", + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("duplicate webhook 'test-webhook' of type 'MutatingAdmissionWebhook'"), + }, + }, { + name: "rejects bundles with duplicate conversion webhook definitions", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook", + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("duplicate webhook 'test-webhook' of type 'ConversionWebhook'"), + }, + }, { + name: "orders errors by webhook type and name", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-val-webhook-b", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-val-webhook-a", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-val-webhook-a", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-val-webhook-b", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-conv-webhook-b", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-conv-webhook-a", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-conv-webhook-a", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-conv-webhook-b", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-mute-webhook-b", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-mute-webhook-a", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-mute-webhook-a", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-mute-webhook-b", + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("duplicate webhook 'test-conv-webhook-a' of type 'ConversionWebhook'"), + errors.New("duplicate webhook 'test-conv-webhook-b' of type 'ConversionWebhook'"), + errors.New("duplicate webhook 'test-mute-webhook-a' of type 'MutatingAdmissionWebhook'"), + errors.New("duplicate webhook 'test-mute-webhook-b' of type 'MutatingAdmissionWebhook'"), + errors.New("duplicate webhook 'test-val-webhook-a' of type 'ValidatingAdmissionWebhook'"), + errors.New("duplicate webhook 'test-val-webhook-b' of type 'ValidatingAdmissionWebhook'"), + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := validators.CheckWebhookNameUniqueness(tc.bundle) + require.Equal(t, tc.expectedErrs, errs) + }) + } +} + +func Test_CheckConversionWebhooksReferenceOwnedCRDs(t *testing.T) { + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + expectedErrs []error + }{ + { + name: "accepts bundles without webhook definitions", + bundle: &render.RegistryV1{}, + }, { + name: "accepts bundles without conversion webhook definitions", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-val-webhook", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-mute-webhook", + }, + ), + ), + }, + }, { + name: "accepts bundles with conversion webhooks that reference owned CRDs", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithOwnedCRDs( + v1alpha1.CRDDescription{Name: "some.crd.something"}, + v1alpha1.CRDDescription{Name: "another.crd.something"}, + ), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook", + ConversionCRDs: []string{ + "some.crd.something", + "another.crd.something", + }, + }, + ), + ), + }, + }, { + name: "rejects bundles with conversion webhooks that reference existing CRDs that are not owned", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithOwnedCRDs( + v1alpha1.CRDDescription{Name: "some.crd.something"}, + ), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook", + ConversionCRDs: []string{ + "some.crd.something", + "another.crd.something", + }, + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("conversion webhook 'test-webhook' references custom resource definition 'another.crd.something' not owned bundle"), + }, + }, { + name: "errors are ordered by webhook name and CRD name", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithOwnedCRDs( + v1alpha1.CRDDescription{Name: "b.crd.something"}, + ), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook-b", + ConversionCRDs: []string{ + "b.crd.something", + }, + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook-a", + ConversionCRDs: []string{ + "c.crd.something", + "a.crd.something", + }, + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook-c", + ConversionCRDs: []string{ + "a.crd.something", + "d.crd.something", + }, + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("conversion webhook 'test-webhook-a' references custom resource definition 'a.crd.something' not owned bundle"), + errors.New("conversion webhook 'test-webhook-a' references custom resource definition 'c.crd.something' not owned bundle"), + errors.New("conversion webhook 'test-webhook-c' references custom resource definition 'a.crd.something' not owned bundle"), + errors.New("conversion webhook 'test-webhook-c' references custom resource definition 'd.crd.something' not owned bundle"), + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := validators.CheckConversionWebhooksReferenceOwnedCRDs(tc.bundle) + require.Equal(t, tc.expectedErrs, errs) + }) + } +} + +func Test_CheckConversionWebhookCRDReferenceUniqueness(t *testing.T) { + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + expectedErrs []error + }{ + { + name: "accepts bundles without webhook definitions", + bundle: &render.RegistryV1{}, + expectedErrs: []error{}, + }, + { + name: "accepts bundles without conversion webhook definitions", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "test-val-webhook", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "test-mute-webhook", + }, + ), + ), + }, + expectedErrs: []error{}, + }, + { + name: "accepts bundles with conversion webhooks that reference different CRDs", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithOwnedCRDs( + v1alpha1.CRDDescription{Name: "some.crd.something"}, + v1alpha1.CRDDescription{Name: "another.crd.something"}, + ), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook", + ConversionCRDs: []string{ + "some.crd.something", + }, + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook-2", + ConversionCRDs: []string{ + "another.crd.something", + }, + }, + ), + ), + }, + expectedErrs: []error{}, + }, + { + name: "rejects bundles with conversion webhooks that reference the same CRD", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithOwnedCRDs( + v1alpha1.CRDDescription{Name: "some.crd.something"}, + ), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook", + ConversionCRDs: []string{ + "some.crd.something", + }, + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook-two", + ConversionCRDs: []string{ + "some.crd.something", + }, + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("conversion webhooks [test-webhook,test-webhook-two] reference same custom resource definition 'some.crd.something'"), + }, + }, + { + name: "errors are ordered by CRD name and webhook names", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithOwnedCRDs( + v1alpha1.CRDDescription{Name: "b.crd.something"}, + ), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook-b", + ConversionCRDs: []string{ + "b.crd.something", + "a.crd.something", + }, + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook-a", + ConversionCRDs: []string{ + "d.crd.something", + "a.crd.something", + "b.crd.something", + }, + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "test-webhook-c", + ConversionCRDs: []string{ + "b.crd.something", + "d.crd.something", + }, + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("conversion webhooks [test-webhook-a,test-webhook-b] reference same custom resource definition 'a.crd.something'"), + errors.New("conversion webhooks [test-webhook-a,test-webhook-b,test-webhook-c] reference same custom resource definition 'b.crd.something'"), + errors.New("conversion webhooks [test-webhook-a,test-webhook-c] reference same custom resource definition 'd.crd.something'"), + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := validators.CheckConversionWebhookCRDReferenceUniqueness(tc.bundle) + require.Equal(t, tc.expectedErrs, errs) + }) + } +} diff --git a/internal/operator-controller/rukpak/util/testing/testing.go b/internal/operator-controller/rukpak/util/testing/testing.go index 4a247dc07..89a620871 100644 --- a/internal/operator-controller/rukpak/util/testing/testing.go +++ b/internal/operator-controller/rukpak/util/testing/testing.go @@ -58,6 +58,12 @@ func WithInstallModeSupportFor(installModeType ...v1alpha1.InstallModeType) CSVO } } +func WithWebhookDefinitions(webhookDefinitions ...v1alpha1.WebhookDescription) CSVOption { + return func(csv *v1alpha1.ClusterServiceVersion) { + csv.Spec.WebhookDefinitions = webhookDefinitions + } +} + func MakeCSV(opts ...CSVOption) v1alpha1.ClusterServiceVersion { csv := v1alpha1.ClusterServiceVersion{ TypeMeta: metav1.TypeMeta{ From 33f665c127b66507460f3bf7fef77b2d0e03f18d Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Mon, 7 Apr 2025 10:52:50 +0200 Subject: [PATCH 02/11] Add webhook support Signed-off-by: Per Goncalves da Silva --- go.mod | 14 +- go.sum | 44 +- .../operator-controller/features/features.go | 10 + .../rukpak/convert/registryv1.go | 15 +- .../rukpak/convert/registryv1_test.go | 48 + .../rukpak/render/certprovider.go | 78 ++ .../rukpak/render/certprovider_test.go | 117 ++ .../render/certproviders/certmanager.go | 117 ++ .../render/certproviders/certmanager_test.go | 158 +++ .../rukpak/render/generators/generators.go | 357 ++++- .../render/generators/generators_test.go | 1143 ++++++++++++++++- .../rukpak/render/generators/resources.go | 76 ++ .../render/generators/resources_test.go | 68 + .../rukpak/render/render.go | 1 + .../rukpak/util/testing.go | 59 - .../rukpak/util/testing/testing.go | 32 + .../rukpak/util/testing_test.go | 188 --- .../operator-controller/rukpak/util/util.go | 17 + 18 files changed, 2243 insertions(+), 299 deletions(-) create mode 100644 internal/operator-controller/rukpak/render/certprovider.go create mode 100644 internal/operator-controller/rukpak/render/certprovider_test.go create mode 100644 internal/operator-controller/rukpak/render/certproviders/certmanager.go create mode 100644 internal/operator-controller/rukpak/render/certproviders/certmanager_test.go delete mode 100644 internal/operator-controller/rukpak/util/testing.go delete mode 100644 internal/operator-controller/rukpak/util/testing_test.go diff --git a/go.mod b/go.mod index ecbbad8c6..c6a32369c 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/BurntSushi/toml v1.5.0 github.com/Masterminds/semver/v3 v3.3.1 github.com/blang/semver/v4 v4.0.0 + github.com/cert-manager/cert-manager v1.17.1 github.com/containerd/containerd v1.7.27 github.com/containers/image/v5 v5.35.0 github.com/fsnotify/fsnotify v1.9.0 @@ -45,7 +46,7 @@ require ( require ( k8s.io/component-helpers v0.32.3 // indirect - k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f // indirect + k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7 // indirect ) require ( @@ -61,7 +62,7 @@ require ( github.com/Microsoft/hcsshim v0.12.9 // indirect github.com/VividCortex/ewma v1.2.0 // indirect github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d // indirect - github.com/antlr4-go/antlr/v4 v4.13.0 // indirect + github.com/antlr4-go/antlr/v4 v4.13.1 // indirect github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect @@ -96,7 +97,7 @@ require ( github.com/evanphx/json-patch v5.9.0+incompatible // indirect github.com/evanphx/json-patch/v5 v5.9.11 // indirect github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect - github.com/fatih/color v1.15.0 // indirect + github.com/fatih/color v1.16.0 // indirect github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fxamacker/cbor/v2 v2.7.0 // indirect github.com/go-errors/errors v1.4.2 // indirect @@ -128,7 +129,7 @@ require ( github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect github.com/google/uuid v1.6.0 // indirect github.com/gorilla/mux v1.8.1 // indirect - github.com/gorilla/websocket v1.5.0 // indirect + github.com/gorilla/websocket v1.5.3 // indirect github.com/gosuri/uitable v0.0.4 // indirect github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.25.1 // indirect @@ -218,7 +219,7 @@ require ( go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.59.0 // indirect go.opentelemetry.io/otel v1.34.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.33.0 // indirect - go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.32.0 // indirect + go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.33.0 // indirect go.opentelemetry.io/otel/metric v1.34.0 // indirect go.opentelemetry.io/otel/sdk v1.34.0 // indirect go.opentelemetry.io/otel/trace v1.34.0 // indirect @@ -243,7 +244,8 @@ require ( k8s.io/controller-manager v0.32.3 // indirect k8s.io/kubectl v0.32.3 // indirect oras.land/oras-go v1.2.5 // indirect - sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.0 // indirect + sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.1 // indirect + sigs.k8s.io/gateway-api v1.1.0 // indirect sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect sigs.k8s.io/kustomize/api v0.18.0 // indirect sigs.k8s.io/kustomize/kyaml v0.18.1 // indirect diff --git a/go.sum b/go.sum index 7ceeafb6e..3191df3e1 100644 --- a/go.sum +++ b/go.sum @@ -34,8 +34,8 @@ github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d h1:licZJFw2RwpH github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d/go.mod h1:asat636LX7Bqt5lYEZ27JNDcqxfjdBQuJ/MM4CN/Lzo= github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc= github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= -github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= -github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g= +github.com/antlr4-go/antlr/v4 v4.13.1 h1:SqQKkuVZ+zWkMMNkjy5FZe5mr5WURWnlpmOuzYWrPrQ= +github.com/antlr4-go/antlr/v4 v4.13.1/go.mod h1:GKmUxMtwp6ZgGwZSva4eWPC5mS6vUAmOABFgjdkM7Nw= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= @@ -51,6 +51,8 @@ github.com/bshuster-repo/logrus-logstash-hook v1.0.0/go.mod h1:zsTqEiSzDgAa/8GZR github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= +github.com/cert-manager/cert-manager v1.17.1 h1:Aig+lWMoLsmpGd9TOlTvO4t0Ah3D+/vGB37x/f+ZKt0= +github.com/cert-manager/cert-manager v1.17.1/go.mod h1:zeG4D+AdzqA7hFMNpYCJgcQ2VOfFNBa+Jzm3kAwiDU4= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/chai2010/gettext-go v1.0.2 h1:1Lwwip6Q2QGsAdl/ZKPCwTe9fe0CjlUbqj5bFNSjIRk= @@ -140,8 +142,8 @@ github.com/evanphx/json-patch/v5 v5.9.11 h1:/8HVnzMq13/3x9TPvjG08wUGqBTmZBsCWzjT github.com/evanphx/json-patch/v5 v5.9.11/go.mod h1:3j+LviiESTElxA4p3EMKAB9HXj3/XEtnUf6OZxqIQTM= github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f h1:Wl78ApPPB2Wvf/TIe2xdyJxTlb6obmF18d8QdkxNDu4= github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f/go.mod h1:OSYXu++VVOHnXeitef/D8n/6y4QV8uLHSFXX4NeXMGc= -github.com/fatih/color v1.15.0 h1:kOqh6YHBtK8aywxGerMG2Eq3H6Qgoqeo13Bk2Mv/nBs= -github.com/fatih/color v1.15.0/go.mod h1:0h5ZqXfHYED7Bhv2ZJamyIOUej9KtShiJESRwBDUSsw= +github.com/fatih/color v1.16.0 h1:zmkK9Ngbjj+K0yRhTVONQh1p/HknKYSlNT+vZCzyokM= +github.com/fatih/color v1.16.0/go.mod h1:fL2Sau1YI5c0pdGEVCbKQbLXB6edEj1ZgiY4NijnWvE= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/foxcpp/go-mockdns v1.1.0 h1:jI0rD8M0wuYAxL7r/ynTrCQQq0BVqfB99Vgk7DlmewI= @@ -260,8 +262,8 @@ github.com/gorilla/handlers v1.5.2 h1:cLTUSsNkgcwhgRqvCNmdbRWG0A3N4F+M2nWKdScwyE github.com/gorilla/handlers v1.5.2/go.mod h1:dX+xVpaxdSw+q0Qek8SSsl3dfMk3jNddUkMzo0GtH0w= github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= -github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc= -github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= +github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= +github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/gosuri/uitable v0.0.4 h1:IG2xLKRvErL3uhY6e1BylFzG+aJiwQviDDTfOKeKTpY= github.com/gosuri/uitable v0.0.4/go.mod h1:tKR86bXuXPZazfOTG1FIzvjIdXzd0mo4Vtn16vt0PJo= github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 h1:+ngKgrYPPJrOjhax5N+uePQ0Fh1Z7PheYoUI/0nzkPA= @@ -340,8 +342,8 @@ github.com/mattn/go-sqlite3 v1.14.22/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxU github.com/mattn/go-sqlite3 v1.14.28 h1:ThEiQrnbtumT+QMknw63Befp/ce/nUPgBPMlRFEum7A= github.com/mattn/go-sqlite3 v1.14.28/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= -github.com/miekg/dns v1.1.58 h1:ca2Hdkz+cDg/7eNF6V56jjzuZ4aCAE+DbVkILdQWG/4= -github.com/miekg/dns v1.1.58/go.mod h1:Ypv+3b/KadlvW9vJfXOTf300O4UqaHFzFCuHz+rPkBY= +github.com/miekg/dns v1.1.62 h1:cN8OuEF1/x5Rq6Np+h1epln8OiyPWV+lROx9LxcGgIQ= +github.com/miekg/dns v1.1.62/go.mod h1:mvDlcItzm+br7MToIKqkglaGhlFMHJ9DTNNWONWXbNQ= github.com/miekg/pkcs11 v1.1.1 h1:Ugu9pdy6vAYku5DEpVWVFPYnzV+bxB+iRdbuFSu7TvU= github.com/miekg/pkcs11 v1.1.1/go.mod h1:XsNlhZGX73bx86s2hdc/FuaLm2CPZJemRLMA+WTFxgs= github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw= @@ -528,12 +530,12 @@ github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9dec github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= go.etcd.io/bbolt v1.4.0 h1:TU77id3TnN/zKr7CO/uk+fBCwF2jGcMuw2B/FMAzYIk= go.etcd.io/bbolt v1.4.0/go.mod h1:AsD+OCi/qPN1giOX1aiLAha3o1U8rAz65bvN4j0sRuk= -go.etcd.io/etcd/api/v3 v3.5.16 h1:WvmyJVbjWqK4R1E+B12RRHz3bRGy9XVfh++MgbN+6n0= -go.etcd.io/etcd/api/v3 v3.5.16/go.mod h1:1P4SlIP/VwkDmGo3OlOD7faPeP8KDIFhqvciH5EfN28= -go.etcd.io/etcd/client/pkg/v3 v3.5.16 h1:ZgY48uH6UvB+/7R9Yf4x574uCO3jIx0TRDyetSfId3Q= -go.etcd.io/etcd/client/pkg/v3 v3.5.16/go.mod h1:V8acl8pcEK0Y2g19YlOV9m9ssUe6MgiDSobSoaBAM0E= -go.etcd.io/etcd/client/v3 v3.5.16 h1:sSmVYOAHeC9doqi0gv7v86oY/BTld0SEFGaxsU9eRhE= -go.etcd.io/etcd/client/v3 v3.5.16/go.mod h1:X+rExSGkyqxvu276cr2OwPLBaeqFu1cIl4vmRjAD/50= +go.etcd.io/etcd/api/v3 v3.5.17 h1:cQB8eb8bxwuxOilBpMJAEo8fAONyrdXTHUNcMd8yT1w= +go.etcd.io/etcd/api/v3 v3.5.17/go.mod h1:d1hvkRuXkts6PmaYk2Vrgqbv7H4ADfAKhyJqHNLJCB4= +go.etcd.io/etcd/client/pkg/v3 v3.5.17 h1:XxnDXAWq2pnxqx76ljWwiQ9jylbpC4rvkAeRVOUKKVw= +go.etcd.io/etcd/client/pkg/v3 v3.5.17/go.mod h1:4DqK1TKacp/86nJk4FLQqo6Mn2vvQFBmruW3pP14H/w= +go.etcd.io/etcd/client/v3 v3.5.17 h1:o48sINNeWz5+pjy/Z0+HKpj/xSnBkuVhVvXkjEXbqZY= +go.etcd.io/etcd/client/v3 v3.5.17/go.mod h1:j2d4eXTHWkT2ClBgnnEPm/Wuu7jsqku41v9DZ3OtjQo= go.mongodb.org/mongo-driver v1.14.0 h1:P98w8egYRjYe3XDjxhYJagTokP/H6HzlsnojRgZRd80= go.mongodb.org/mongo-driver v1.14.0/go.mod h1:Vzb0Mk/pa7e6cWw85R4F/endUC3u0U9jGcNU603k65c= go.opencensus.io v0.24.0 h1:y73uSU6J157QMP2kn2r30vwW1A2W2WFwSCGnAVxeaD0= @@ -560,8 +562,8 @@ go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.32.0 h1:t/Q go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.32.0/go.mod h1:Rl61tySSdcOJWoEgYZVtmnKdA0GeKrSqkHC1t+91CH8= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.33.0 h1:Vh5HayB/0HHfOQA7Ctx69E/Y/DcQSMPpKANYVMQ7fBA= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.33.0/go.mod h1:cpgtDBaqD/6ok/UG0jT15/uKjAY8mRA53diogHBg3UI= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.32.0 h1:9kV11HXBHZAvuPUZxmMWrH8hZn/6UnHX4K0mu36vNsU= -go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.32.0/go.mod h1:JyA0FHXe22E1NeNiHmVp7kFHglnexDQ7uRWDiiJ1hKQ= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.33.0 h1:5pojmb1U1AogINhN3SurB+zm/nIcusopeBNp42f45QM= +go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.33.0/go.mod h1:57gTHJSE5S1tqg+EKsLPlTWhpHMsWlVmer+LA926XiA= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.33.0 h1:wpMfgF8E1rkrT1Z6meFh1NDtownE9Ii3n3X2GJYjsaU= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.33.0/go.mod h1:wAy0T/dUbs468uOlkT31xjvqQgEVXv58BRFWEgn5v/0= go.opentelemetry.io/otel/exporters/prometheus v0.54.0 h1:rFwzp68QMgtzu9PgP3jm9XaMICI6TsofWWPcBDKwlsU= @@ -795,8 +797,8 @@ k8s.io/controller-manager v0.32.3 h1:jBxZnQ24k6IMeWLyxWZmpa3QVS7ww+osAIzaUY/jqyc k8s.io/controller-manager v0.32.3/go.mod h1:out1L3DZjE/p7JG0MoMMIaQGWIkt3c+pKaswqSHgKsI= k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk= k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE= -k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f h1:GA7//TjRY9yWGy1poLzYYJJ4JRdzg3+O6e8I+e+8T5Y= -k8s.io/kube-openapi v0.0.0-20241105132330-32ad38e42d3f/go.mod h1:R/HEjbvWI0qdfb8viZUeVZm0X6IZnxAydC7YU42CMw4= +k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7 h1:hcha5B1kVACrLujCKLbr8XWMxCxzQx42DY8QKYJrDLg= +k8s.io/kube-openapi v0.0.0-20241212222426-2c72e554b1e7/go.mod h1:GewRfANuJ70iYzvn+i4lezLDAFzvjxZYK1gn1lWcfas= k8s.io/kubectl v0.32.3 h1:VMi584rbboso+yjfv0d8uBHwwxbC438LKq+dXd5tOAI= k8s.io/kubectl v0.32.3/go.mod h1:6Euv2aso5GKzo/UVMacV6C7miuyevpfI91SvBvV9Zdg= k8s.io/kubernetes v1.32.3 h1:2A58BlNME8NwsMawmnM6InYo3Jf35Nw5G79q46kXwoA= @@ -807,10 +809,12 @@ oras.land/oras-go v1.2.5 h1:XpYuAwAb0DfQsunIyMfeET92emK8km3W4yEzZvUbsTo= oras.land/oras-go v1.2.5/go.mod h1:PuAwRShRZCsZb7g8Ar3jKKQR/2A/qN+pkYxIOd/FAoo= oras.land/oras-go/v2 v2.5.0 h1:o8Me9kLY74Vp5uw07QXPiitjsw7qNXi8Twd+19Zf02c= oras.land/oras-go/v2 v2.5.0/go.mod h1:z4eisnLP530vwIOUOJeBIj0aGI0L1C3d53atvCBqZHg= -sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.0 h1:CPT0ExVicCzcpeN4baWEV2ko2Z/AsiZgEdwgcfwLgMo= -sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.0/go.mod h1:Ve9uj1L+deCXFrPOk1LpFXqTg7LCFzFso6PA48q/XZw= +sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.1 h1:uOuSLOMBWkJH0TWa9X6l+mj5nZdm6Ay6Bli8HL8rNfk= +sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.1/go.mod h1:Ve9uj1L+deCXFrPOk1LpFXqTg7LCFzFso6PA48q/XZw= sigs.k8s.io/controller-runtime v0.20.4 h1:X3c+Odnxz+iPTRobG4tp092+CvBU9UK0t/bRf+n0DGU= sigs.k8s.io/controller-runtime v0.20.4/go.mod h1:xg2XB0K5ShQzAgsoujxuKN4LNXR2LfwwHsPj7Iaw+XY= +sigs.k8s.io/gateway-api v1.1.0 h1:DsLDXCi6jR+Xz8/xd0Z1PYl2Pn0TyaFMOPPZIj4inDM= +sigs.k8s.io/gateway-api v1.1.0/go.mod h1:ZH4lHrL2sDi0FHZ9jjneb8kKnGzFWyrTya35sWUTrRs= sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 h1:gBQPwqORJ8d8/YNZWEjoZs7npUVDpVXUUOFfW6CgAqE= sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8/go.mod h1:mdzfpAEoE6DHQEN0uh9ZbOCuHbLK5wOm7dK4ctXE9Tg= sigs.k8s.io/kustomize/api v0.18.0 h1:hTzp67k+3NEVInwz5BHyzc9rGxIauoXferXyjv5lWPo= diff --git a/internal/operator-controller/features/features.go b/internal/operator-controller/features/features.go index 2e9083735..ea4f68a85 100644 --- a/internal/operator-controller/features/features.go +++ b/internal/operator-controller/features/features.go @@ -14,6 +14,7 @@ const ( PreflightPermissions featuregate.Feature = "PreflightPermissions" SingleOwnNamespaceInstallSupport featuregate.Feature = "SingleOwnNamespaceInstallSupport" SyntheticPermissions featuregate.Feature = "SyntheticPermissions" + WebhookSupport featuregate.Feature = "WebhookSupport" ) var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -41,6 +42,15 @@ var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.Feature PreRelease: featuregate.Alpha, LockToDefault: false, }, + + // WebhookSupport enables support for installing + // registry+v1 cluster extensions that include validating, + // mutating, and/or conversion webhooks + WebhookSupport: { + Default: false, + PreRelease: featuregate.Alpha, + LockToDefault: false, + }, } var OperatorControllerFeatureGate featuregate.MutableFeatureGate = featuregate.NewFeatureGate() diff --git a/internal/operator-controller/rukpak/convert/registryv1.go b/internal/operator-controller/rukpak/convert/registryv1.go index 1417239fd..fcb6a1d5c 100644 --- a/internal/operator-controller/rukpak/convert/registryv1.go +++ b/internal/operator-controller/rukpak/convert/registryv1.go @@ -20,8 +20,10 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-registry/alpha/property" + "github.com/operator-framework/operator-controller/internal/operator-controller/features" registry "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/operator-registry" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/certproviders" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/generators" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators" ) @@ -220,6 +222,10 @@ var PlainConverter = Converter{ generators.BundleCRDGenerator, generators.BundleAdditionalResourcesGenerator, generators.BundleCSVDeploymentGenerator, + generators.BundleValidatingWebhookResourceGenerator, + generators.BundleMutatingWebhookResourceGenerator, + generators.BundleWebhookServiceResourceGenerator, + generators.CertProviderResourceGenerator, }, }, } @@ -257,11 +263,16 @@ func (c Converter) Convert(rv1 render.RegistryV1, installNamespace string, targe return nil, fmt.Errorf("apiServiceDefintions are not supported") } - if len(rv1.CSV.Spec.WebhookDefinitions) > 0 { + if !features.OperatorControllerFeatureGate.Enabled(features.WebhookSupport) && len(rv1.CSV.Spec.WebhookDefinitions) > 0 { return nil, fmt.Errorf("webhookDefinitions are not supported") } - objs, err := c.BundleRenderer.Render(rv1, installNamespace, render.WithTargetNamespaces(targetNamespaces...)) + objs, err := c.BundleRenderer.Render( + rv1, + installNamespace, + render.WithTargetNamespaces(targetNamespaces...), + certproviders.WithCertManagerCertificateProvider(), + ) if err != nil { return nil, err } diff --git a/internal/operator-controller/rukpak/convert/registryv1_test.go b/internal/operator-controller/rukpak/convert/registryv1_test.go index dffb15cb4..49296ae56 100644 --- a/internal/operator-controller/rukpak/convert/registryv1_test.go +++ b/internal/operator-controller/rukpak/convert/registryv1_test.go @@ -18,12 +18,14 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-registry/alpha/property" + "github.com/operator-framework/operator-controller/internal/operator-controller/features" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators" @@ -560,6 +562,52 @@ func TestRegistryV1SuiteGenerateNoWebhooks(t *testing.T) { require.Nil(t, plainBundle) } +func TestRegistryV1SuiteGenerateWebhooks_WebhookSupportFGEnabled(t *testing.T) { + featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.WebhookSupport, true) + t.Log("RegistryV1 Suite Convert") + t.Log("It should generate objects successfully based on target namespaces") + + t.Log("It should enforce limitations") + t.Log("It should allow bundles with webhooks") + t.Log("By creating a registry v1 bundle") + registryv1Bundle := render.RegistryV1{ + PackageName: "testPkg", + CRDs: []apiextensionsv1.CustomResourceDefinition{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-webhook.package-with-webhooks.io", + }, + }, + }, + CSV: MakeCSV( + WithName("testCSV"), + WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), + WithOwnedCRDs( + v1alpha1.CRDDescription{ + Name: "fake-webhook.package-with-webhooks.io", + }, + ), + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "some-deployment", + }, + ), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + ConversionCRDs: []string{"fake-webhook.package-with-webhooks.io"}, + DeploymentName: "some-deployment", + }, + ), + ), + } + + t.Log("By converting to plain") + plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, []string{metav1.NamespaceAll}) + require.NoError(t, err) + require.NotNil(t, plainBundle) +} + func TestRegistryV1SuiteGenerateNoAPISerciceDefinitions(t *testing.T) { t.Log("RegistryV1 Suite Convert") t.Log("It should generate objects successfully based on target namespaces") diff --git a/internal/operator-controller/rukpak/render/certprovider.go b/internal/operator-controller/rukpak/render/certprovider.go new file mode 100644 index 000000000..f3920a4c7 --- /dev/null +++ b/internal/operator-controller/rukpak/render/certprovider.go @@ -0,0 +1,78 @@ +package render + +import ( + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" +) + +// CertificateProvider encapsulate the creation and modification of object for certificate provisioning +// in Kubernetes by vendors such as CertManager or the OpenshiftServiceCA operator +type CertificateProvider interface { + InjectCABundle(obj client.Object, cfg CertificateProvisionerConfig) error + AdditionalObjects(cfg CertificateProvisionerConfig) ([]unstructured.Unstructured, error) + GetCertSecretInfo(cfg CertificateProvisionerConfig) CertSecretInfo +} + +// CertSecretInfo contains describes the certificate secret resource information such as name and +// certificate and private key keys +type CertSecretInfo struct { + SecretName string + CertificateKey string + PrivateKeyKey string +} + +// CertificateProvisionerConfig contains the necessary information for a CertificateProvider +// to correctly generate and modify object for certificate injection and automation +type CertificateProvisionerConfig struct { + WebhookServiceName string + CertName string + Namespace string + CertProvider CertificateProvider +} + +// CertificateProvisioner uses a CertificateProvider to modify and generate objects based on its +// CertificateProvisionerConfig +type CertificateProvisioner CertificateProvisionerConfig + +func (c CertificateProvisioner) InjectCABundle(obj client.Object) error { + if c.CertProvider == nil { + return nil + } + return c.CertProvider.InjectCABundle(obj, CertificateProvisionerConfig(c)) +} + +func (c CertificateProvisioner) AdditionalObjects() ([]unstructured.Unstructured, error) { + if c.CertProvider == nil { + return nil, nil + } + return c.CertProvider.AdditionalObjects(CertificateProvisionerConfig(c)) +} + +func (c CertificateProvisioner) GetCertSecretInfo() *CertSecretInfo { + if c.CertProvider == nil { + return nil + } + info := c.CertProvider.GetCertSecretInfo(CertificateProvisionerConfig(c)) + return &info +} + +func CertProvisionerFor(deploymentName string, opts Options) CertificateProvisioner { + // maintaining parity with OLMv0 naming + // See https://github.com/operator-framework/operator-lifecycle-manager/blob/658a6a60de8315f055f54aa7e50771ee4daa8983/pkg/controller/install/webhook.go#L254 + webhookServiceName := util.ObjectNameForBaseAndSuffix(strings.ReplaceAll(deploymentName, ".", "-"), "service") + + // maintaining parity with cert secret name in OLMv0 + // See https://github.com/operator-framework/operator-lifecycle-manager/blob/658a6a60de8315f055f54aa7e50771ee4daa8983/pkg/controller/install/certresources.go#L151 + certName := util.ObjectNameForBaseAndSuffix(webhookServiceName, "cert") + + return CertificateProvisioner{ + CertProvider: opts.CertificateProvider, + WebhookServiceName: webhookServiceName, + Namespace: opts.InstallNamespace, + CertName: certName, + } +} diff --git a/internal/operator-controller/rukpak/render/certprovider_test.go b/internal/operator-controller/rukpak/render/certprovider_test.go new file mode 100644 index 000000000..293a86b46 --- /dev/null +++ b/internal/operator-controller/rukpak/render/certprovider_test.go @@ -0,0 +1,117 @@ +package render_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" +) + +func Test_CertificateProvisioner_WithoutCertProvider(t *testing.T) { + provisioner := &render.CertificateProvisioner{ + WebhookServiceName: "webhook", + CertName: "cert", + Namespace: "namespace", + CertProvider: nil, + } + + require.NoError(t, provisioner.InjectCABundle(&corev1.Secret{})) + require.Nil(t, provisioner.GetCertSecretInfo()) + + objs, err := provisioner.AdditionalObjects() + require.Nil(t, objs) + require.NoError(t, err) +} + +func Test_CertificateProvisioner_WithCertProvider(t *testing.T) { + fakeProvider := &FakeCertProvider{ + InjectCABundleFn: func(obj client.Object, cfg render.CertificateProvisionerConfig) error { + obj.SetName("some-name") + return nil + }, + AdditionalObjectsFn: func(cfg render.CertificateProvisionerConfig) ([]unstructured.Unstructured, error) { + return []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{})}, nil + }, + GetCertSecretInfoFn: func(cfg render.CertificateProvisionerConfig) render.CertSecretInfo { + return render.CertSecretInfo{ + SecretName: "some-secret", + PrivateKeyKey: "some-key", + CertificateKey: "another-key", + } + }, + } + provisioner := &render.CertificateProvisioner{ + WebhookServiceName: "webhook", + CertName: "cert", + Namespace: "namespace", + CertProvider: fakeProvider, + } + + svc := &corev1.Service{} + require.NoError(t, provisioner.InjectCABundle(svc)) + require.Equal(t, "some-name", svc.GetName()) + + objs, err := provisioner.AdditionalObjects() + require.NoError(t, err) + require.Equal(t, []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{})}, objs) + + require.Equal(t, &render.CertSecretInfo{ + SecretName: "some-secret", + PrivateKeyKey: "some-key", + CertificateKey: "another-key", + }, provisioner.GetCertSecretInfo()) +} + +func Test_CertificateProvisioner_Errors(t *testing.T) { + fakeProvider := &FakeCertProvider{ + InjectCABundleFn: func(obj client.Object, cfg render.CertificateProvisionerConfig) error { + return fmt.Errorf("some error") + }, + AdditionalObjectsFn: func(cfg render.CertificateProvisionerConfig) ([]unstructured.Unstructured, error) { + return nil, fmt.Errorf("some other error") + }, + } + provisioner := &render.CertificateProvisioner{ + WebhookServiceName: "webhook", + CertName: "cert", + Namespace: "namespace", + CertProvider: fakeProvider, + } + + err := provisioner.InjectCABundle(&corev1.Service{}) + require.Error(t, err) + require.Contains(t, err.Error(), "some error") + + objs, err := provisioner.AdditionalObjects() + require.Error(t, err) + require.Contains(t, err.Error(), "some other error") + require.Nil(t, objs) +} + +func Test_CertProvisionerFor(t *testing.T) { + fakeProvider := &FakeCertProvider{} + prov := render.CertProvisionerFor("my.deployment.thing", render.Options{ + InstallNamespace: "my-namespace", + CertificateProvider: fakeProvider, + }) + + require.Equal(t, prov.CertProvider, fakeProvider) + require.Equal(t, "my-deployment-thing-service", prov.WebhookServiceName) + require.Equal(t, "my-deployment-thing-service-cert", prov.CertName) + require.Equal(t, "my-namespace", prov.Namespace) +} + +func Test_CertProvisionerFor_ExtraLargeName_MoreThan63Chars(t *testing.T) { + prov := render.CertProvisionerFor("my.object.thing.has.a.really.really.really.really.really.long.name", render.Options{}) + + require.Len(t, prov.WebhookServiceName, 63) + require.Len(t, prov.CertName, 63) + require.Equal(t, "my-object-thing-has-a-really-really-really-really-reall-service", prov.WebhookServiceName) + require.Equal(t, "my-object-thing-has-a-really-really-really-really-reall-se-cert", prov.CertName) +} diff --git a/internal/operator-controller/rukpak/render/certproviders/certmanager.go b/internal/operator-controller/rukpak/render/certproviders/certmanager.go new file mode 100644 index 000000000..b2d32d7bd --- /dev/null +++ b/internal/operator-controller/rukpak/render/certproviders/certmanager.go @@ -0,0 +1,117 @@ +package certproviders + +import ( + "errors" + "fmt" + + certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" +) + +const ( + certManagerInjectCAAnnotation = "cert-manager.io/inject-ca-from" +) + +func WithCertManagerCertificateProvider() render.Option { + return func(o *render.Options) { + o.CertificateProvider = CertManagerCertificateProvider{} + } +} + +var _ render.CertificateProvider = (*CertManagerCertificateProvider)(nil) + +type CertManagerCertificateProvider struct{} + +func (p CertManagerCertificateProvider) InjectCABundle(obj client.Object, cfg render.CertificateProvisionerConfig) error { + switch obj.(type) { + case *admissionregistrationv1.ValidatingWebhookConfiguration: + p.addCAInjectionAnnotation(obj, cfg.Namespace, cfg.CertName) + case *admissionregistrationv1.MutatingWebhookConfiguration: + p.addCAInjectionAnnotation(obj, cfg.Namespace, cfg.CertName) + case *apiextensionsv1.CustomResourceDefinition: + p.addCAInjectionAnnotation(obj, cfg.Namespace, cfg.CertName) + } + return nil +} + +func (p CertManagerCertificateProvider) GetCertSecretInfo(cfg render.CertificateProvisionerConfig) render.CertSecretInfo { + return render.CertSecretInfo{ + SecretName: cfg.CertName, + PrivateKeyKey: "tls.key", + CertificateKey: "tls.crt", + } +} + +func (p CertManagerCertificateProvider) AdditionalObjects(cfg render.CertificateProvisionerConfig) ([]unstructured.Unstructured, error) { + var ( + objs []unstructured.Unstructured + errs []error + ) + + issuer := &certmanagerv1.Issuer{ + TypeMeta: metav1.TypeMeta{ + APIVersion: certmanagerv1.SchemeGroupVersion.String(), + Kind: "Issuer", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: util.ObjectNameForBaseAndSuffix(cfg.CertName, "selfsigned-issuer"), + Namespace: cfg.Namespace, + }, + Spec: certmanagerv1.IssuerSpec{ + IssuerConfig: certmanagerv1.IssuerConfig{ + SelfSigned: &certmanagerv1.SelfSignedIssuer{}, + }, + }, + } + issuerObj, err := util.ToUnstructured(issuer) + if err != nil { + errs = append(errs, err) + } else { + objs = append(objs, *issuerObj) + } + + certificate := &certmanagerv1.Certificate{ + TypeMeta: metav1.TypeMeta{ + APIVersion: certmanagerv1.SchemeGroupVersion.String(), + Kind: "Certificate", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: cfg.CertName, + Namespace: cfg.Namespace, + }, + Spec: certmanagerv1.CertificateSpec{ + SecretName: cfg.CertName, + Usages: []certmanagerv1.KeyUsage{certmanagerv1.UsageServerAuth}, + DNSNames: []string{fmt.Sprintf("%s.%s.svc", cfg.WebhookServiceName, cfg.Namespace)}, + IssuerRef: certmanagermetav1.ObjectReference{ + Name: issuer.GetName(), + }, + }, + } + certObj, err := util.ToUnstructured(certificate) + if err != nil { + errs = append(errs, err) + } else { + objs = append(objs, *certObj) + } + + if len(errs) > 0 { + return nil, errors.Join(errs...) + } + return objs, nil +} + +func (p CertManagerCertificateProvider) addCAInjectionAnnotation(obj client.Object, certNamespace string, certName string) { + injectionAnnotation := map[string]string{ + certManagerInjectCAAnnotation: fmt.Sprintf("%s/%s", certNamespace, certName), + } + obj.SetAnnotations(util.MergeMaps(obj.GetAnnotations(), injectionAnnotation)) +} diff --git a/internal/operator-controller/rukpak/render/certproviders/certmanager_test.go b/internal/operator-controller/rukpak/render/certproviders/certmanager_test.go new file mode 100644 index 000000000..08570d12f --- /dev/null +++ b/internal/operator-controller/rukpak/render/certproviders/certmanager_test.go @@ -0,0 +1,158 @@ +package certproviders_test + +import ( + "testing" + + certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + "github.com/stretchr/testify/require" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/certproviders" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" +) + +func Test_CertManagerProvider_InjectCABundle(t *testing.T) { + for _, tc := range []struct { + name string + obj client.Object + cfg render.CertificateProvisionerConfig + expectedObj client.Object + }{ + { + name: "injects certificate annotation in validating webhook configuration", + obj: &admissionregistrationv1.ValidatingWebhookConfiguration{}, + cfg: render.CertificateProvisionerConfig{ + WebhookServiceName: "webhook-service", + Namespace: "namespace", + CertName: "cert-name", + }, + expectedObj: &admissionregistrationv1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "cert-manager.io/inject-ca-from": "namespace/cert-name", + }, + }, + }, + }, + { + name: "injects certificate annotation in mutating webhook configuration", + obj: &admissionregistrationv1.MutatingWebhookConfiguration{}, + cfg: render.CertificateProvisionerConfig{ + WebhookServiceName: "webhook-service", + Namespace: "namespace", + CertName: "cert-name", + }, + expectedObj: &admissionregistrationv1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "cert-manager.io/inject-ca-from": "namespace/cert-name", + }, + }, + }, + }, + { + name: "injects certificate annotation in custom resource definition", + obj: &apiextensionsv1.CustomResourceDefinition{}, + cfg: render.CertificateProvisionerConfig{ + WebhookServiceName: "webhook-service", + Namespace: "namespace", + CertName: "cert-name", + }, + expectedObj: &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "cert-manager.io/inject-ca-from": "namespace/cert-name", + }, + }, + }, + }, + { + name: "ignores other objects", + obj: &corev1.Service{}, + cfg: render.CertificateProvisionerConfig{ + WebhookServiceName: "webhook-service", + Namespace: "namespace", + CertName: "cert-name", + }, + expectedObj: &corev1.Service{}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + certProvier := certproviders.CertManagerCertificateProvider{} + require.NoError(t, certProvier.InjectCABundle(tc.obj, tc.cfg)) + require.Equal(t, tc.expectedObj, tc.obj) + }) + } +} + +func Test_CertManagerProvider_AdditionalObjects(t *testing.T) { + certProvier := certproviders.CertManagerCertificateProvider{} + objs, err := certProvier.AdditionalObjects(render.CertificateProvisionerConfig{ + WebhookServiceName: "webhook-service", + Namespace: "namespace", + CertName: "cert-name", + }) + require.NoError(t, err) + require.Equal(t, []unstructured.Unstructured{ + toUnstructured(t, &certmanagerv1.Issuer{ + TypeMeta: metav1.TypeMeta{ + APIVersion: certmanagerv1.SchemeGroupVersion.String(), + Kind: "Issuer", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "cert-name-selfsigned-issuer", + Namespace: "namespace", + }, + Spec: certmanagerv1.IssuerSpec{ + IssuerConfig: certmanagerv1.IssuerConfig{ + SelfSigned: &certmanagerv1.SelfSignedIssuer{}, + }, + }, + }), + toUnstructured(t, &certmanagerv1.Certificate{ + TypeMeta: metav1.TypeMeta{ + APIVersion: certmanagerv1.SchemeGroupVersion.String(), + Kind: "Certificate", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "cert-name", + Namespace: "namespace", + }, + Spec: certmanagerv1.CertificateSpec{ + SecretName: "cert-name", + Usages: []certmanagerv1.KeyUsage{certmanagerv1.UsageServerAuth}, + DNSNames: []string{"webhook-service.namespace.svc"}, + IssuerRef: certmanagermetav1.ObjectReference{ + Name: "cert-name-selfsigned-issuer", + }, + }, + }), + }, objs) +} + +func Test_CertManagerProvider_GetCertSecretInfo(t *testing.T) { + certProvier := certproviders.CertManagerCertificateProvider{} + certInfo := certProvier.GetCertSecretInfo(render.CertificateProvisionerConfig{ + WebhookServiceName: "webhook-service", + Namespace: "namespace", + CertName: "cert-name", + }) + require.Equal(t, render.CertSecretInfo{ + SecretName: "cert-name", + PrivateKeyKey: "tls.key", + CertificateKey: "tls.crt", + }, certInfo) +} + +func toUnstructured(t *testing.T, obj client.Object) unstructured.Unstructured { + u, err := util.ToUnstructured(obj) + require.NoError(t, err) + return *u +} diff --git a/internal/operator-controller/rukpak/render/generators/generators.go b/internal/operator-controller/rukpak/render/generators/generators.go index dfc73a2ab..31f8857cd 100644 --- a/internal/operator-controller/rukpak/render/generators/generators.go +++ b/internal/operator-controller/rukpak/render/generators/generators.go @@ -3,20 +3,39 @@ package generators import ( "cmp" "fmt" + "maps" + "slices" + "strconv" "strings" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/operator-framework/api/pkg/operators/v1alpha1" registrybundle "github.com/operator-framework/operator-registry/pkg/lib/bundle" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) +var certVolumeMounts = map[string]corev1.VolumeMount{ + "apiservice-cert": { + Name: "apiservice-cert", + MountPath: "/apiserver.local.config/certificates", + }, + "webhook-cert": { + Name: "webhook-cert", + MountPath: "/tmp/k8s-webhook-server/serving-certs", + }, +} + // BundleCSVRBACResourceGenerator generates all ServiceAccounts, ClusterRoles, ClusterRoleBindings, Roles, RoleBindings // defined in the RegistryV1 bundle's cluster service version (CSV) var BundleCSVRBACResourceGenerator = render.ResourceGenerators{ @@ -34,6 +53,13 @@ func BundleCSVDeploymentGenerator(rv1 *render.RegistryV1, opts render.Options) ( if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } + + // collect deployments that service webhooks + webhookDeployments := sets.Set[string]{} + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + webhookDeployments.Insert(wh.DeploymentName) + } + objs := make([]client.Object, 0, len(rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs)) for _, depSpec := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { // Add CSV annotations to template annotations @@ -50,14 +76,19 @@ func BundleCSVDeploymentGenerator(rv1 *render.RegistryV1, opts render.Options) ( // See https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/deployment.go#L177-L180 depSpec.Spec.RevisionHistoryLimit = ptr.To(int32(1)) - objs = append(objs, - CreateDeploymentResource( - depSpec.Name, - opts.InstallNamespace, - WithDeploymentSpec(depSpec.Spec), - WithLabels(depSpec.Label), - ), + deploymentResource := CreateDeploymentResource( + depSpec.Name, + opts.InstallNamespace, + WithDeploymentSpec(depSpec.Spec), + WithLabels(depSpec.Label), ) + + secretInfo := render.CertProvisionerFor(depSpec.Name, opts).GetCertSecretInfo() + if webhookDeployments.Has(depSpec.Name) && secretInfo != nil { + addCertVolumesToDeployment(deploymentResource, *secretInfo) + } + + objs = append(objs, deploymentResource) } return objs, nil } @@ -171,14 +202,66 @@ func BundleCSVServiceAccountGenerator(rv1 *render.RegistryV1, opts render.Option return objs, nil } -// BundleCRDGenerator generates CustomResourceDefinition resources from the registry+v1 bundle -func BundleCRDGenerator(rv1 *render.RegistryV1, _ render.Options) ([]client.Object, error) { +// BundleCRDGenerator generates CustomResourceDefinition resources from the registry+v1 bundle. If the CRD is referenced +// by any conversion webhook defined in the bundle's cluster service version spec, the CRD is modified +// by the CertificateProvider in opts to add any annotations or modifications necessary for certificate injection. +func BundleCRDGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { if rv1 == nil { return nil, fmt.Errorf("bundle cannot be nil") } + + // collect deployments to crds with conversion webhooks + crdToDeploymentMap := map[string]v1alpha1.WebhookDescription{} + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + if wh.Type != v1alpha1.ConversionWebhook { + continue + } + for _, crdName := range wh.ConversionCRDs { + if _, ok := crdToDeploymentMap[crdName]; ok { + return nil, fmt.Errorf("custom resource definition '%s' is referenced by multiple conversion webhook definitions", crdName) + } + crdToDeploymentMap[crdName] = wh + } + } + objs := make([]client.Object, 0, len(rv1.CRDs)) for _, crd := range rv1.CRDs { - objs = append(objs, crd.DeepCopy()) + cp := crd.DeepCopy() + if cw, ok := crdToDeploymentMap[crd.Name]; ok { + // OLMv0 behaviour parity + // See https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/webhook.go#L232 + if crd.Spec.PreserveUnknownFields { + return nil, fmt.Errorf("custom resource definition '%s' must have .spec.preserveUnknownFields set to false to let API Server call webhook to do the conversion", crd.Name) + } + + // OLMv0 behaviour parity + // https://github.com/operator-framework/operator-lifecycle-manager/blob/dfd0b2bea85038d3c0d65348bc812d297f16b8d2/pkg/controller/install/webhook.go#L242 + conversionWebhookPath := "/" + if cw.WebhookPath != nil { + conversionWebhookPath = *cw.WebhookPath + } + + certProvisioner := render.CertProvisionerFor(cw.DeploymentName, opts) + cp.Spec.Conversion = &apiextensionsv1.CustomResourceConversion{ + Strategy: apiextensionsv1.WebhookConverter, + Webhook: &apiextensionsv1.WebhookConversion{ + ClientConfig: &apiextensionsv1.WebhookClientConfig{ + Service: &apiextensionsv1.ServiceReference{ + Namespace: opts.InstallNamespace, + Name: certProvisioner.WebhookServiceName, + Path: &conversionWebhookPath, + Port: &cw.ContainerPort, + }, + }, + ConversionReviewVersions: cw.AdmissionReviewVersions, + }, + } + + if err := certProvisioner.InjectCABundle(cp); err != nil { + return nil, err + } + } + objs = append(objs, cp) } return objs, nil } @@ -206,6 +289,260 @@ func BundleAdditionalResourcesGenerator(rv1 *render.RegistryV1, opts render.Opti return objs, nil } +// BundleValidatingWebhookResourceGenerator generates ValidatingAdmissionWebhookConfiguration resources based on +// the bundle's cluster service version spec. The resource is modified by the CertificateProvider in opts +// to add any annotations or modifications necessary for certificate injection. +func BundleValidatingWebhookResourceGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + if rv1 == nil { + return nil, fmt.Errorf("bundle cannot be nil") + } + + //nolint:prealloc + var objs []client.Object + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + if wh.Type != v1alpha1.ValidatingAdmissionWebhook { + continue + } + certProvisioner := render.CertProvisionerFor(wh.DeploymentName, opts) + webhookResource := CreateValidatingWebhookConfigurationResource( + wh.GenerateName, + opts.InstallNamespace, + WithValidatingWebhooks( + admissionregistrationv1.ValidatingWebhook{ + Name: wh.GenerateName, + Rules: wh.Rules, + FailurePolicy: wh.FailurePolicy, + MatchPolicy: wh.MatchPolicy, + ObjectSelector: wh.ObjectSelector, + SideEffects: wh.SideEffects, + TimeoutSeconds: wh.TimeoutSeconds, + AdmissionReviewVersions: wh.AdmissionReviewVersions, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: opts.InstallNamespace, + Name: certProvisioner.WebhookServiceName, + Path: wh.WebhookPath, + Port: &wh.ContainerPort, + }, + }, + }, + ), + ) + if err := certProvisioner.InjectCABundle(webhookResource); err != nil { + return nil, err + } + objs = append(objs, webhookResource) + } + return objs, nil +} + +// BundleMutatingWebhookResourceGenerator generates MutatingAdmissionWebhookConfiguration resources based on +// the bundle's cluster service version spec. The resource is modified by the CertificateProvider in opts +// to add any annotations or modifications necessary for certificate injection. +func BundleMutatingWebhookResourceGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + if rv1 == nil { + return nil, fmt.Errorf("bundle cannot be nil") + } + + //nolint:prealloc + var objs []client.Object + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + if wh.Type != v1alpha1.MutatingAdmissionWebhook { + continue + } + certProvisioner := render.CertProvisionerFor(wh.DeploymentName, opts) + webhookResource := CreateMutatingWebhookConfigurationResource( + wh.GenerateName, + opts.InstallNamespace, + WithMutatingWebhooks( + admissionregistrationv1.MutatingWebhook{ + Name: wh.GenerateName, + Rules: wh.Rules, + FailurePolicy: wh.FailurePolicy, + MatchPolicy: wh.MatchPolicy, + ObjectSelector: wh.ObjectSelector, + SideEffects: wh.SideEffects, + TimeoutSeconds: wh.TimeoutSeconds, + AdmissionReviewVersions: wh.AdmissionReviewVersions, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: opts.InstallNamespace, + Name: certProvisioner.WebhookServiceName, + Path: wh.WebhookPath, + Port: &wh.ContainerPort, + }, + }, + ReinvocationPolicy: wh.ReinvocationPolicy, + }, + ), + ) + if err := certProvisioner.InjectCABundle(webhookResource); err != nil { + return nil, err + } + objs = append(objs, webhookResource) + } + return objs, nil +} + +// BundleWebhookServiceResourceGenerator generates Service resources based that support the webhooks defined in +// the bundle's cluster service version spec. The resource is modified by the CertificateProvider in opts +// to add any annotations or modifications necessary for certificate injection. +func BundleWebhookServiceResourceGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + if rv1 == nil { + return nil, fmt.Errorf("bundle cannot be nil") + } + + // collect webhook service ports + webhookServicePortsByDeployment := map[string]sets.Set[corev1.ServicePort]{} + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + if _, ok := webhookServicePortsByDeployment[wh.DeploymentName]; !ok { + webhookServicePortsByDeployment[wh.DeploymentName] = sets.Set[corev1.ServicePort]{} + } + webhookServicePortsByDeployment[wh.DeploymentName].Insert(getWebhookServicePort(wh)) + } + + objs := make([]client.Object, 0, len(webhookServicePortsByDeployment)) + for _, deploymentSpec := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { + if _, ok := webhookServicePortsByDeployment[deploymentSpec.Name]; !ok { + continue + } + + servicePorts := webhookServicePortsByDeployment[deploymentSpec.Name] + ports := servicePorts.UnsortedList() + slices.SortStableFunc(ports, func(a, b corev1.ServicePort) int { + return cmp.Or(cmp.Compare(a.Port, b.Port), cmp.Compare(a.TargetPort.IntValue(), b.TargetPort.IntValue())) + }) + + var labelSelector map[string]string + if deploymentSpec.Spec.Selector != nil { + labelSelector = deploymentSpec.Spec.Selector.MatchLabels + } + + certProvisioner := render.CertProvisionerFor(deploymentSpec.Name, opts) + serviceResource := CreateServiceResource( + certProvisioner.WebhookServiceName, + opts.InstallNamespace, + WithServiceSpec( + corev1.ServiceSpec{ + Ports: ports, + Selector: labelSelector, + }, + ), + ) + + if err := certProvisioner.InjectCABundle(serviceResource); err != nil { + return nil, err + } + objs = append(objs, serviceResource) + } + + return objs, nil +} + +// CertProviderResourceGenerator generates any resources necessary for the CertificateProvider +// in opts to function correctly, e.g. Issuer or Certificate resources. +func CertProviderResourceGenerator(rv1 *render.RegistryV1, opts render.Options) ([]client.Object, error) { + deploymentsWithWebhooks := sets.Set[string]{} + + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + deploymentsWithWebhooks.Insert(wh.DeploymentName) + } + + var objs []client.Object + for _, depName := range deploymentsWithWebhooks.UnsortedList() { + certCfg := render.CertProvisionerFor(depName, opts) + certObjs, err := certCfg.AdditionalObjects() + if err != nil { + return nil, err + } + for _, certObj := range certObjs { + objs = append(objs, &certObj) + } + } + return objs, nil +} + func saNameOrDefault(saName string) string { return cmp.Or(saName, "default") } + +func getWebhookServicePort(wh v1alpha1.WebhookDescription) corev1.ServicePort { + containerPort := int32(443) + if wh.ContainerPort > 0 { + containerPort = wh.ContainerPort + } + + targetPort := intstr.FromInt32(containerPort) + if wh.TargetPort != nil { + targetPort = *wh.TargetPort + } + + return corev1.ServicePort{ + Name: strconv.Itoa(int(containerPort)), + Port: containerPort, + TargetPort: targetPort, + } +} + +func addCertVolumesToDeployment(dep *appsv1.Deployment, certSecretInfo render.CertSecretInfo) { + // update pod volumes + dep.Spec.Template.Spec.Volumes = slices.Concat( + slices.DeleteFunc(dep.Spec.Template.Spec.Volumes, func(v corev1.Volume) bool { + _, ok := certVolumeMounts[v.Name] + return ok + }), + []corev1.Volume{ + { + Name: "apiservice-cert", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: certSecretInfo.SecretName, + Items: []corev1.KeyToPath{ + { + Key: certSecretInfo.CertificateKey, + Path: "apiserver.crt", + }, + { + Key: certSecretInfo.PrivateKeyKey, + Path: "apiserver.key", + }, + }, + }, + }, + }, { + Name: "webhook-cert", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: certSecretInfo.SecretName, + Items: []corev1.KeyToPath{ + { + Key: certSecretInfo.CertificateKey, + Path: "tls.crt", + }, + { + Key: certSecretInfo.PrivateKeyKey, + Path: "tls.key", + }, + }, + }, + }, + }, + }, + ) + + // update container volume mounts + for i := range dep.Spec.Template.Spec.Containers { + dep.Spec.Template.Spec.Containers[i].VolumeMounts = slices.Concat( + slices.DeleteFunc(dep.Spec.Template.Spec.Containers[i].VolumeMounts, func(v corev1.VolumeMount) bool { + _, ok := certVolumeMounts[v.Name] + return ok + }), + slices.SortedFunc( + maps.Values(certVolumeMounts), + func(a corev1.VolumeMount, b corev1.VolumeMount) int { + return cmp.Compare(a.Name, b.Name) + }, + ), + ) + } +} diff --git a/internal/operator-controller/rukpak/render/generators/generators_test.go b/internal/operator-controller/rukpak/render/generators/generators_test.go index d3151f829..d47c84abb 100644 --- a/internal/operator-controller/rukpak/render/generators/generators_test.go +++ b/internal/operator-controller/rukpak/render/generators/generators_test.go @@ -8,13 +8,14 @@ import ( "testing" "github.com/stretchr/testify/require" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -22,7 +23,7 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/generators" - . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" + . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" ) func Test_BundleCSVRBACResourceGenerator_HasCorrectGenerators(t *testing.T) { @@ -175,6 +176,160 @@ func Test_BundleCSVDeploymentGenerator_Succeeds(t *testing.T) { } } +func Test_BundleCSVDeploymentGenerator_WithCertWithCertProvider_Succeeds(t *testing.T) { + fakeProvider := FakeCertProvider{ + GetCertSecretInfoFn: func(cfg render.CertificateProvisionerConfig) render.CertSecretInfo { + return render.CertSecretInfo{ + SecretName: "some-secret", + CertificateKey: "some-cert-key", + PrivateKeyKey: "some-private-key-key", + } + }, + } + + bundle := &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + DeploymentName: "deployment-one", + }), + // deployment must have a referencing webhook (or owned apiservice) definition to trigger cert secret + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "deployment-one", + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: "apiservice-cert", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + { + Name: "some-other-mount", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + // expect webhook-cert volume to be injected + }, + Containers: []corev1.Container{ + { + Name: "container-1", + VolumeMounts: []corev1.VolumeMount{ + // expect apiservice-cert volume to be injected + { + Name: "webhook-cert", + MountPath: "/webhook-cert-path", + }, { + Name: "some-other-mount", + MountPath: "/some/other/mount/path", + }, + }, + }, + { + Name: "container-2", + // expect cert volumes to be injected + }, + }, + }, + }, + }, + }, + ), + ), + } + + objs, err := generators.BundleCSVDeploymentGenerator(bundle, render.Options{ + InstallNamespace: "install-namespace", + CertificateProvider: fakeProvider, + }) + require.NoError(t, err) + require.Len(t, objs, 1) + + deployment := objs[0].(*appsv1.Deployment) + require.NotNil(t, deployment) + + require.Equal(t, []corev1.Volume{ + { + Name: "some-other-mount", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + { + Name: "apiservice-cert", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "some-secret", + Items: []corev1.KeyToPath{ + { + Key: "some-cert-key", + Path: "apiserver.crt", + }, + { + Key: "some-private-key-key", + Path: "apiserver.key", + }, + }, + }, + }, + }, { + Name: "webhook-cert", + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "some-secret", + Items: []corev1.KeyToPath{ + { + Key: "some-cert-key", + Path: "tls.crt", + }, + { + Key: "some-private-key-key", + Path: "tls.key", + }, + }, + }, + }, + }, + }, deployment.Spec.Template.Spec.Volumes) + require.Equal(t, []corev1.Container{ + { + Name: "container-1", + VolumeMounts: []corev1.VolumeMount{ + { + Name: "some-other-mount", + MountPath: "/some/other/mount/path", + }, + { + Name: "apiservice-cert", + MountPath: "/apiserver.local.config/certificates", + }, + { + Name: "webhook-cert", + MountPath: "/tmp/k8s-webhook-server/serving-certs", + }, + }, + }, + { + Name: "container-2", + VolumeMounts: []corev1.VolumeMount{ + { + Name: "apiservice-cert", + MountPath: "/apiserver.local.config/certificates", + }, + { + Name: "webhook-cert", + MountPath: "/tmp/k8s-webhook-server/serving-certs", + }, + }, + }, + }, deployment.Spec.Template.Spec.Containers) +} + func Test_BundleCSVDeploymentGenerator_FailsOnNil(t *testing.T) { objs, err := generators.BundleCSVDeploymentGenerator(nil, render.Options{}) require.Nil(t, objs) @@ -1118,6 +1273,164 @@ func Test_BundleCRDGenerator_Succeeds(t *testing.T) { }, objs) } +func Test_BundleCRDGenerator_WithConversionWebhook_Succeeds(t *testing.T) { + opts := render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{""}, + } + + bundle := &render.RegistryV1{ + CRDs: []apiextensionsv1.CustomResourceDefinition{ + {ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "crd-two"}}, + }, + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + WebhookPath: ptr.To("/some/path"), + ContainerPort: 8443, + AdmissionReviewVersions: []string{"v1", "v1beta1"}, + ConversionCRDs: []string{"crd-one"}, + DeploymentName: "some-deployment", + }, + v1alpha1.WebhookDescription{ + // should use / as WebhookPath by default + Type: v1alpha1.ConversionWebhook, + ContainerPort: 8443, + AdmissionReviewVersions: []string{"v1", "v1beta1"}, + ConversionCRDs: []string{"crd-two"}, + DeploymentName: "some-deployment", + }, + ), + ), + } + + objs, err := generators.BundleCRDGenerator(bundle, opts) + require.NoError(t, err) + require.Equal(t, []client.Object{ + &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "crd-one", + }, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Conversion: &apiextensionsv1.CustomResourceConversion{ + Strategy: apiextensionsv1.WebhookConverter, + Webhook: &apiextensionsv1.WebhookConversion{ + ClientConfig: &apiextensionsv1.WebhookClientConfig{ + Service: &apiextensionsv1.ServiceReference{ + Namespace: "install-namespace", + Name: "some-deployment-service", + Path: ptr.To("/some/path"), + Port: ptr.To(int32(8443)), + }, + }, + ConversionReviewVersions: []string{"v1", "v1beta1"}, + }, + }, + }, + }, + &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "crd-two", + }, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Conversion: &apiextensionsv1.CustomResourceConversion{ + Strategy: apiextensionsv1.WebhookConverter, + Webhook: &apiextensionsv1.WebhookConversion{ + ClientConfig: &apiextensionsv1.WebhookClientConfig{ + Service: &apiextensionsv1.ServiceReference{ + Namespace: "install-namespace", + Name: "some-deployment-service", + Path: ptr.To("/"), + Port: ptr.To(int32(8443)), + }, + }, + ConversionReviewVersions: []string{"v1", "v1beta1"}, + }, + }, + }, + }, + }, objs) +} + +func Test_BundleCRDGenerator_WithConversionWebhook_Fails(t *testing.T) { + opts := render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{""}, + } + + bundle := &render.RegistryV1{ + CRDs: []apiextensionsv1.CustomResourceDefinition{ + { + ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + PreserveUnknownFields: true, + }, + }, + }, + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + WebhookPath: ptr.To("/some/path"), + ContainerPort: 8443, + AdmissionReviewVersions: []string{"v1", "v1beta1"}, + ConversionCRDs: []string{"crd-one"}, + DeploymentName: "some-deployment", + }, + ), + ), + } + + objs, err := generators.BundleCRDGenerator(bundle, opts) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "must have .spec.preserveUnknownFields set to false to let API Server call webhook to do the conversion") +} + +func Test_BundleCRDGenerator_WithCertProvider_Succeeds(t *testing.T) { + fakeProvider := FakeCertProvider{ + InjectCABundleFn: func(obj client.Object, cfg render.CertificateProvisionerConfig) error { + obj.SetAnnotations(map[string]string{ + "cert-provider": "annotation", + }) + return nil + }, + } + + opts := render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{""}, + CertificateProvider: fakeProvider, + } + + bundle := &render.RegistryV1{ + CRDs: []apiextensionsv1.CustomResourceDefinition{ + {ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}}, + {ObjectMeta: metav1.ObjectMeta{Name: "crd-two"}}, + }, + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + DeploymentName: "my-deployment", + ConversionCRDs: []string{ + "crd-one", + }, + }, + ), + ), + } + + objs, err := generators.BundleCRDGenerator(bundle, opts) + require.NoError(t, err) + require.Len(t, objs, 2) + require.Equal(t, map[string]string{ + "cert-provider": "annotation", + }, objs[0].GetAnnotations()) +} + func Test_BundleCRDGenerator_FailsOnNil(t *testing.T) { objs, err := generators.BundleCRDGenerator(nil, render.Options{}) require.Nil(t, objs) @@ -1132,7 +1445,7 @@ func Test_BundleAdditionalResourcesGenerator_Succeeds(t *testing.T) { bundle := &render.RegistryV1{ Others: []unstructured.Unstructured{ - toUnstructured(t, + *ToUnstructuredT(t, &corev1.Service{ TypeMeta: metav1.TypeMeta{ Kind: "Service", @@ -1143,7 +1456,7 @@ func Test_BundleAdditionalResourcesGenerator_Succeeds(t *testing.T) { }, }, ), - toUnstructured(t, + *ToUnstructuredT(t, &rbacv1.ClusterRole{ TypeMeta: metav1.TypeMeta{ Kind: "ClusterRole", @@ -1169,15 +1482,817 @@ func Test_BundleAdditionalResourcesGenerator_FailsOnNil(t *testing.T) { require.Contains(t, err.Error(), "bundle cannot be nil") } -func toUnstructured(t *testing.T, obj client.Object) unstructured.Unstructured { - gvk := obj.GetObjectKind().GroupVersionKind() +func Test_BundleValidatingWebhookResourceGenerator_Succeeds(t *testing.T) { + fakeProvider := FakeCertProvider{ + InjectCABundleFn: func(obj client.Object, cfg render.CertificateProvisionerConfig) error { + obj.SetAnnotations(map[string]string{ + "cert-provider": "annotation", + }) + return nil + }, + } + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + opts render.Options + expectedResources []client.Object + }{ + { + name: "generates validating webhook configuration resources described in the bundle's cluster service version", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "my-webhook", + DeploymentName: "my-deployment", + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.OperationAll, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{""}, + Resources: []string{"namespaces"}, + }, + }, + }, + FailurePolicy: ptr.To(admissionregistrationv1.Fail), + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + SideEffects: ptr.To(admissionregistrationv1.SideEffectClassNone), + TimeoutSeconds: ptr.To(int32(1)), + AdmissionReviewVersions: []string{ + "v1beta1", + "v1beta2", + }, + WebhookPath: ptr.To("/webhook-path"), + ContainerPort: 443, + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &admissionregistrationv1.ValidatingWebhookConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "ValidatingWebhookConfiguration", + APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "my-webhook-", + Namespace: "install-namespace", + }, + Webhooks: []admissionregistrationv1.ValidatingWebhook{ + { + Name: "my-webhook", + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.OperationAll, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{""}, + Resources: []string{"namespaces"}, + }, + }, + }, + FailurePolicy: ptr.To(admissionregistrationv1.Fail), + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + SideEffects: ptr.To(admissionregistrationv1.SideEffectClassNone), + TimeoutSeconds: ptr.To(int32(1)), + AdmissionReviewVersions: []string{ + "v1beta1", + "v1beta2", + }, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: "install-namespace", + Name: "my-deployment-service", + Path: ptr.To("/webhook-path"), + Port: ptr.To(int32(443)), + }, + }, + }, + }, + }, + }, + }, + { + name: "generates validating webhook configuration resources with certificate provider modifications", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "my-webhook", + DeploymentName: "my-deployment", + ContainerPort: 443, + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + CertificateProvider: fakeProvider, + }, + expectedResources: []client.Object{ + &admissionregistrationv1.ValidatingWebhookConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "ValidatingWebhookConfiguration", + APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "my-webhook-", + Namespace: "install-namespace", + Annotations: map[string]string{ + "cert-provider": "annotation", + }, + }, + Webhooks: []admissionregistrationv1.ValidatingWebhook{ + { + Name: "my-webhook", + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: "install-namespace", + Name: "my-deployment-service", + Port: ptr.To(int32(443)), + }, + }, + }, + }, + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + objs, err := generators.BundleValidatingWebhookResourceGenerator(tc.bundle, tc.opts) + require.NoError(t, err) + require.Equal(t, tc.expectedResources, objs) + }) + } +} + +func Test_BundleValidatingWebhookResourceGenerator_FailsOnNil(t *testing.T) { + objs, err := generators.BundleValidatingWebhookResourceGenerator(nil, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "bundle cannot be nil") +} - var u unstructured.Unstructured - uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) - require.NoError(t, err) - unstructured.RemoveNestedField(uObj, "metadata", "creationTimestamp") - unstructured.RemoveNestedField(uObj, "status") - u.Object = uObj - u.SetGroupVersionKind(gvk) - return u +func Test_BundleMutatingWebhookResourceGenerator_Succeeds(t *testing.T) { + fakeProvider := FakeCertProvider{ + InjectCABundleFn: func(obj client.Object, cfg render.CertificateProvisionerConfig) error { + obj.SetAnnotations(map[string]string{ + "cert-provider": "annotation", + }) + return nil + }, + } + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + opts render.Options + expectedResources []client.Object + }{ + { + name: "generates validating webhook configuration resources described in the bundle's cluster service version", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "my-webhook", + DeploymentName: "my-deployment", + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.OperationAll, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{""}, + Resources: []string{"namespaces"}, + }, + }, + }, + FailurePolicy: ptr.To(admissionregistrationv1.Fail), + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + SideEffects: ptr.To(admissionregistrationv1.SideEffectClassNone), + TimeoutSeconds: ptr.To(int32(1)), + AdmissionReviewVersions: []string{ + "v1beta1", + "v1beta2", + }, + WebhookPath: ptr.To("/webhook-path"), + ContainerPort: 443, + ReinvocationPolicy: ptr.To(admissionregistrationv1.IfNeededReinvocationPolicy), + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &admissionregistrationv1.MutatingWebhookConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "MutatingWebhookConfiguration", + APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "my-webhook-", + Namespace: "install-namespace", + }, + Webhooks: []admissionregistrationv1.MutatingWebhook{ + { + Name: "my-webhook", + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.OperationAll, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{""}, + Resources: []string{"namespaces"}, + }, + }, + }, + FailurePolicy: ptr.To(admissionregistrationv1.Fail), + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + SideEffects: ptr.To(admissionregistrationv1.SideEffectClassNone), + TimeoutSeconds: ptr.To(int32(1)), + AdmissionReviewVersions: []string{ + "v1beta1", + "v1beta2", + }, + ReinvocationPolicy: ptr.To(admissionregistrationv1.IfNeededReinvocationPolicy), + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: "install-namespace", + Name: "my-deployment-service", + Path: ptr.To("/webhook-path"), + Port: ptr.To(int32(443)), + }, + }, + }, + }, + }, + }, + }, + { + name: "generates validating webhook configuration resources with certificate provider modifications", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "my-webhook", + DeploymentName: "my-deployment", + ContainerPort: 443, + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + CertificateProvider: fakeProvider, + }, + expectedResources: []client.Object{ + &admissionregistrationv1.MutatingWebhookConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "MutatingWebhookConfiguration", + APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "my-webhook-", + Namespace: "install-namespace", + Annotations: map[string]string{ + "cert-provider": "annotation", + }, + }, + Webhooks: []admissionregistrationv1.MutatingWebhook{ + { + Name: "my-webhook", + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: "install-namespace", + Name: "my-deployment-service", + Port: ptr.To(int32(443)), + }, + }, + }, + }, + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + objs, err := generators.BundleMutatingWebhookResourceGenerator(tc.bundle, tc.opts) + require.NoError(t, err) + require.Equal(t, tc.expectedResources, objs) + }) + } +} + +func Test_BundleMutatingWebhookResourceGenerator_FailsOnNil(t *testing.T) { + objs, err := generators.BundleMutatingWebhookResourceGenerator(nil, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "bundle cannot be nil") +} + +func Test_BundleWebhookServiceResourceGenerator_Succeeds(t *testing.T) { + fakeProvider := FakeCertProvider{ + InjectCABundleFn: func(obj client.Object, cfg render.CertificateProvisionerConfig) error { + obj.SetAnnotations(map[string]string{ + "cert-provider": "annotation", + }) + return nil + }, + } + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + opts render.Options + expectedResources []client.Object + }{ + { + name: "generates webhook services using container port 443 and target port 443 by default", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "my-deployment", + }), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + DeploymentName: "my-deployment", + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment-service", + Namespace: "install-namespace", + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "443", + Port: int32(443), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 443, + }, + }, + }, + }, + }, + }, + }, + { + name: "generates webhook services using the given container port and setting target port the same as the container port if not given", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "my-deployment", + }), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + DeploymentName: "my-deployment", + ContainerPort: int32(8443), + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment-service", + Namespace: "install-namespace", + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "8443", + Port: int32(8443), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, + }, + }, + }, + }, + }, + { + name: "generates webhook services using given container port of 443 and given target port", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "my-deployment", + }), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + DeploymentName: "my-deployment", + TargetPort: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8080, + }, + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment-service", + Namespace: "install-namespace", + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "443", + Port: int32(443), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8080, + }, + }, + }, + }, + }, + }, + }, + { + name: "generates webhook services using given container port and target port", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "my-deployment", + }), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + DeploymentName: "my-deployment", + ContainerPort: int32(9090), + TargetPort: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 9099, + }, + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment-service", + Namespace: "install-namespace", + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "9090", + Port: int32(9090), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 9099, + }, + }, + }, + }, + }, + }, + }, + { + name: "generates webhook services using referenced deployment defined label selector", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "my-deployment", + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + }, + }), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + DeploymentName: "my-deployment", + ContainerPort: int32(9090), + TargetPort: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 9099, + }, + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment-service", + Namespace: "install-namespace", + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "9090", + Port: int32(9090), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 9099, + }, + }, + }, + Selector: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + }, + { + name: "aggregates all webhook definitions referencing the same deployment into a single service", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "my-deployment", + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + }, + }), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + DeploymentName: "my-deployment", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + DeploymentName: "my-deployment", + ContainerPort: int32(8443), + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + DeploymentName: "my-deployment", + TargetPort: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8080, + }, + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + DeploymentName: "my-deployment", + ContainerPort: int32(9090), + TargetPort: &intstr.IntOrString{ + Type: intstr.Int, + IntVal: 9099, + }, + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment-service", + Namespace: "install-namespace", + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "443", + Port: int32(443), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 443, + }, + }, { + Name: "443", + Port: int32(443), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8080, + }, + }, { + Name: "8443", + Port: int32(8443), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 8443, + }, + }, { + Name: "9090", + Port: int32(9090), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 9099, + }, + }, + }, + Selector: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + }, + { + name: "applies cert provider modifiers to webhook service", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "my-deployment", + }), + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + DeploymentName: "my-deployment", + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + CertificateProvider: fakeProvider, + }, + expectedResources: []client.Object{ + &corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment-service", + Namespace: "install-namespace", + Annotations: map[string]string{ + "cert-provider": "annotation", + }, + }, + Spec: corev1.ServiceSpec{ + Ports: []corev1.ServicePort{ + { + Name: "443", + Port: int32(443), + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 443, + }, + }, + }, + }, + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + objs, err := generators.BundleWebhookServiceResourceGenerator(tc.bundle, tc.opts) + require.NoError(t, err) + require.Equal(t, tc.expectedResources, objs) + }) + } +} + +func Test_BundleWebhookServiceResourceGenerator_FailsOnNil(t *testing.T) { + objs, err := generators.BundleMutatingWebhookResourceGenerator(nil, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "bundle cannot be nil") +} + +func Test_CertProviderResourceGenerator_Succeeds(t *testing.T) { + fakeProvider := FakeCertProvider{ + AdditionalObjectsFn: func(cfg render.CertificateProvisionerConfig) ([]unstructured.Unstructured, error) { + return []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: cfg.CertName, + }, + })}, nil + }, + } + + objs, err := generators.CertProviderResourceGenerator(&render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + // only generate resources for deployments referenced by webhook definitions + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + DeploymentName: "my-deployment", + }, + ), + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{ + Name: "my-deployment", + }, + v1alpha1.StrategyDeploymentSpec{ + Name: "my-other-deployment", + }, + ), + ), + }, render.Options{ + InstallNamespace: "install-namespace", + CertificateProvider: fakeProvider, + }) + require.NoError(t, err) + require.Equal(t, []client.Object{ + ToUnstructuredT(t, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-deployment-service-cert", + }, + }), + }, objs) } diff --git a/internal/operator-controller/rukpak/render/generators/resources.go b/internal/operator-controller/rukpak/render/generators/resources.go index fa925f2f3..ddcfd3e15 100644 --- a/internal/operator-controller/rukpak/render/generators/resources.go +++ b/internal/operator-controller/rukpak/render/generators/resources.go @@ -1,6 +1,9 @@ package generators import ( + "fmt" + + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -84,6 +87,36 @@ func WithLabels(labels map[string]string) func(client.Object) { } } +// WithServiceSpec applies a service spec to a Service resource +func WithServiceSpec(serviceSpec corev1.ServiceSpec) func(client.Object) { + return func(obj client.Object) { + switch o := obj.(type) { + case *corev1.Service: + o.Spec = serviceSpec + } + } +} + +// WithValidatingWebhooks applies validating webhooks to a ValidatingWebhookConfiguration resource +func WithValidatingWebhooks(webhooks ...admissionregistrationv1.ValidatingWebhook) func(client.Object) { + return func(obj client.Object) { + switch o := obj.(type) { + case *admissionregistrationv1.ValidatingWebhookConfiguration: + o.Webhooks = webhooks + } + } +} + +// WithMutatingWebhooks applies mutating webhooks to a MutatingWebhookConfiguration resource +func WithMutatingWebhooks(webhooks ...admissionregistrationv1.MutatingWebhook) func(client.Object) { + return func(obj client.Object) { + switch o := obj.(type) { + case *admissionregistrationv1.MutatingWebhookConfiguration: + o.Webhooks = webhooks + } + } +} + // CreateServiceAccountResource creates a ServiceAccount resource with name 'name', namespace 'namespace', and applying // any ServiceAccount related options in opts func CreateServiceAccountResource(name string, namespace string, opts ...ResourceCreatorOption) *corev1.ServiceAccount { @@ -183,3 +216,46 @@ func CreateDeploymentResource(name string, namespace string, opts ...ResourceCre }, ).(*appsv1.Deployment) } + +func CreateValidatingWebhookConfigurationResource(generateName string, namespace string, opts ...ResourceCreatorOption) *admissionregistrationv1.ValidatingWebhookConfiguration { + return ResourceCreatorOptions(opts).ApplyTo( + &admissionregistrationv1.ValidatingWebhookConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "ValidatingWebhookConfiguration", + APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: fmt.Sprintf("%s-", generateName), + Namespace: namespace, + }, + }, + ).(*admissionregistrationv1.ValidatingWebhookConfiguration) +} + +func CreateMutatingWebhookConfigurationResource(generateName string, namespace string, opts ...ResourceCreatorOption) *admissionregistrationv1.MutatingWebhookConfiguration { + return ResourceCreatorOptions(opts).ApplyTo( + &admissionregistrationv1.MutatingWebhookConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "MutatingWebhookConfiguration", + APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + GenerateName: fmt.Sprintf("%s-", generateName), + Namespace: namespace, + }, + }, + ).(*admissionregistrationv1.MutatingWebhookConfiguration) +} + +func CreateServiceResource(name string, namespace string, opts ...ResourceCreatorOption) *corev1.Service { + return ResourceCreatorOptions(opts).ApplyTo(&corev1.Service{ + TypeMeta: metav1.TypeMeta{ + Kind: "Service", + APIVersion: corev1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + }).(*corev1.Service) +} diff --git a/internal/operator-controller/rukpak/render/generators/resources_test.go b/internal/operator-controller/rukpak/render/generators/resources_test.go index 6aeed1c8f..1eef933e5 100644 --- a/internal/operator-controller/rukpak/render/generators/resources_test.go +++ b/internal/operator-controller/rukpak/render/generators/resources_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/stretchr/testify/require" + admissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -74,6 +75,27 @@ func Test_CreateDeployment(t *testing.T) { require.Equal(t, "my-namespace", deployment.Namespace) } +func Test_CreateService(t *testing.T) { + svc := generators.CreateServiceResource("my-service", "my-namespace") + require.NotNil(t, svc) + require.Equal(t, "my-service", svc.Name) + require.Equal(t, "my-namespace", svc.Namespace) +} + +func Test_CreateValidatingWebhookConfiguration(t *testing.T) { + wh := generators.CreateValidatingWebhookConfigurationResource("my-validating-webhook-configuration", "my-namespace") + require.NotNil(t, wh) + require.Equal(t, "my-validating-webhook-configuration-", wh.GenerateName) + require.Equal(t, "my-namespace", wh.Namespace) +} + +func Test_CreateMutatingWebhookConfiguration(t *testing.T) { + wh := generators.CreateMutatingWebhookConfigurationResource("my-mutating-webhook-configuration", "my-namespace") + require.NotNil(t, wh) + require.Equal(t, "my-mutating-webhook-configuration-", wh.GenerateName) + require.Equal(t, "my-namespace", wh.Namespace) +} + func Test_WithSubjects(t *testing.T) { for _, tc := range []struct { name string @@ -208,3 +230,49 @@ func Test_WithLabels(t *testing.T) { }) } } + +func Test_WithServiceSpec(t *testing.T) { + svc := generators.CreateServiceResource("mysvc", "myns", generators.WithServiceSpec(corev1.ServiceSpec{ + ClusterIP: "1.2.3.4", + })) + require.NotNil(t, svc) + require.Equal(t, corev1.ServiceSpec{ + ClusterIP: "1.2.3.4", + }, svc.Spec) +} + +func Test_WithValidatingWebhook(t *testing.T) { + wh := generators.CreateValidatingWebhookConfigurationResource("mywh", "myns", + generators.WithValidatingWebhooks( + admissionregistrationv1.ValidatingWebhook{ + Name: "wh-one", + }, + admissionregistrationv1.ValidatingWebhook{ + Name: "wh-two", + }, + ), + ) + require.NotNil(t, wh) + require.Equal(t, []admissionregistrationv1.ValidatingWebhook{ + {Name: "wh-one"}, + {Name: "wh-two"}, + }, wh.Webhooks) +} + +func Test_WithMutatingWebhook(t *testing.T) { + wh := generators.CreateMutatingWebhookConfigurationResource("mywh", "myns", + generators.WithMutatingWebhooks( + admissionregistrationv1.MutatingWebhook{ + Name: "wh-one", + }, + admissionregistrationv1.MutatingWebhook{ + Name: "wh-two", + }, + ), + ) + require.NotNil(t, wh) + require.Equal(t, []admissionregistrationv1.MutatingWebhook{ + {Name: "wh-one"}, + {Name: "wh-two"}, + }, wh.Webhooks) +} diff --git a/internal/operator-controller/rukpak/render/render.go b/internal/operator-controller/rukpak/render/render.go index 279320afc..03922e454 100644 --- a/internal/operator-controller/rukpak/render/render.go +++ b/internal/operator-controller/rukpak/render/render.go @@ -66,6 +66,7 @@ type Options struct { InstallNamespace string TargetNamespaces []string UniqueNameGenerator UniqueNameGenerator + CertificateProvider CertificateProvider } func (o *Options) apply(opts ...Option) *Options { diff --git a/internal/operator-controller/rukpak/util/testing.go b/internal/operator-controller/rukpak/util/testing.go deleted file mode 100644 index 4dfc12976..000000000 --- a/internal/operator-controller/rukpak/util/testing.go +++ /dev/null @@ -1,59 +0,0 @@ -package util - -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/operator-framework/api/pkg/operators/v1alpha1" -) - -type CSVOption func(version *v1alpha1.ClusterServiceVersion) - -//nolint:unparam -func WithName(name string) CSVOption { - return func(csv *v1alpha1.ClusterServiceVersion) { - csv.Name = name - } -} - -func WithStrategyDeploymentSpecs(strategyDeploymentSpecs ...v1alpha1.StrategyDeploymentSpec) CSVOption { - return func(csv *v1alpha1.ClusterServiceVersion) { - csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs = strategyDeploymentSpecs - } -} - -func WithAnnotations(annotations map[string]string) CSVOption { - return func(csv *v1alpha1.ClusterServiceVersion) { - csv.Annotations = annotations - } -} - -func WithPermissions(permissions ...v1alpha1.StrategyDeploymentPermissions) CSVOption { - return func(csv *v1alpha1.ClusterServiceVersion) { - csv.Spec.InstallStrategy.StrategySpec.Permissions = permissions - } -} - -func WithClusterPermissions(permissions ...v1alpha1.StrategyDeploymentPermissions) CSVOption { - return func(csv *v1alpha1.ClusterServiceVersion) { - csv.Spec.InstallStrategy.StrategySpec.ClusterPermissions = permissions - } -} - -func WithOwnedCRDs(crdDesc ...v1alpha1.CRDDescription) CSVOption { - return func(csv *v1alpha1.ClusterServiceVersion) { - csv.Spec.CustomResourceDefinitions.Owned = crdDesc - } -} - -func MakeCSV(opts ...CSVOption) v1alpha1.ClusterServiceVersion { - csv := v1alpha1.ClusterServiceVersion{ - TypeMeta: metav1.TypeMeta{ - APIVersion: v1alpha1.SchemeGroupVersion.String(), - Kind: "ClusterServiceVersion", - }, - } - for _, opt := range opts { - opt(&csv) - } - return csv -} diff --git a/internal/operator-controller/rukpak/util/testing/testing.go b/internal/operator-controller/rukpak/util/testing/testing.go index 89a620871..0a4ec84fe 100644 --- a/internal/operator-controller/rukpak/util/testing/testing.go +++ b/internal/operator-controller/rukpak/util/testing/testing.go @@ -1,9 +1,17 @@ package testing import ( + "testing" + + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/api/pkg/operators/v1alpha1" + + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util" ) type CSVOption func(version *v1alpha1.ClusterServiceVersion) @@ -76,3 +84,27 @@ func MakeCSV(opts ...CSVOption) v1alpha1.ClusterServiceVersion { } return csv } + +type FakeCertProvider struct { + InjectCABundleFn func(obj client.Object, cfg render.CertificateProvisionerConfig) error + AdditionalObjectsFn func(cfg render.CertificateProvisionerConfig) ([]unstructured.Unstructured, error) + GetCertSecretInfoFn func(cfg render.CertificateProvisionerConfig) render.CertSecretInfo +} + +func (f FakeCertProvider) InjectCABundle(obj client.Object, cfg render.CertificateProvisionerConfig) error { + return f.InjectCABundleFn(obj, cfg) +} + +func (f FakeCertProvider) AdditionalObjects(cfg render.CertificateProvisionerConfig) ([]unstructured.Unstructured, error) { + return f.AdditionalObjectsFn(cfg) +} + +func (f FakeCertProvider) GetCertSecretInfo(cfg render.CertificateProvisionerConfig) render.CertSecretInfo { + return f.GetCertSecretInfoFn(cfg) +} + +func ToUnstructuredT(t *testing.T, obj client.Object) *unstructured.Unstructured { + u, err := util.ToUnstructured(obj) + require.NoError(t, err) + return u +} diff --git a/internal/operator-controller/rukpak/util/testing_test.go b/internal/operator-controller/rukpak/util/testing_test.go deleted file mode 100644 index 17ca328f8..000000000 --- a/internal/operator-controller/rukpak/util/testing_test.go +++ /dev/null @@ -1,188 +0,0 @@ -package util - -import ( - "testing" - - "github.com/stretchr/testify/require" - rbacv1 "k8s.io/api/rbac/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/operator-framework/api/pkg/operators/v1alpha1" -) - -func Test_MakeCSV(t *testing.T) { - csv := MakeCSV() - require.Equal(t, v1alpha1.ClusterServiceVersion{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterServiceVersion", - APIVersion: v1alpha1.SchemeGroupVersion.String(), - }, - }, csv) -} - -func Test_MakeCSV_WithName(t *testing.T) { - csv := MakeCSV(WithName("some-name")) - require.Equal(t, v1alpha1.ClusterServiceVersion{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterServiceVersion", - APIVersion: v1alpha1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "some-name", - }, - }, csv) -} - -func Test_MakeCSV_WithStrategyDeploymentSpecs(t *testing.T) { - csv := MakeCSV( - WithStrategyDeploymentSpecs( - v1alpha1.StrategyDeploymentSpec{ - Name: "spec-one", - }, - v1alpha1.StrategyDeploymentSpec{ - Name: "spec-two", - }, - ), - ) - - require.Equal(t, v1alpha1.ClusterServiceVersion{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterServiceVersion", - APIVersion: v1alpha1.SchemeGroupVersion.String(), - }, - Spec: v1alpha1.ClusterServiceVersionSpec{ - InstallStrategy: v1alpha1.NamedInstallStrategy{ - StrategySpec: v1alpha1.StrategyDetailsDeployment{ - DeploymentSpecs: []v1alpha1.StrategyDeploymentSpec{ - { - Name: "spec-one", - }, - { - Name: "spec-two", - }, - }, - }, - }, - }, - }, csv) -} - -func Test_MakeCSV_WithPermissions(t *testing.T) { - csv := MakeCSV( - WithPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"secrets"}, - Verbs: []string{"list", "watch"}, - }, - }, - }, - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "", - }, - ), - ) - - require.Equal(t, v1alpha1.ClusterServiceVersion{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterServiceVersion", - APIVersion: v1alpha1.SchemeGroupVersion.String(), - }, - Spec: v1alpha1.ClusterServiceVersionSpec{ - InstallStrategy: v1alpha1.NamedInstallStrategy{ - StrategySpec: v1alpha1.StrategyDetailsDeployment{ - Permissions: []v1alpha1.StrategyDeploymentPermissions{ - { - ServiceAccountName: "service-account", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"secrets"}, - Verbs: []string{"list", "watch"}, - }, - }, - }, - { - ServiceAccountName: "", - }, - }, - }, - }, - }, - }, csv) -} - -func Test_MakeCSV_WithClusterPermissions(t *testing.T) { - csv := MakeCSV( - WithClusterPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"secrets"}, - Verbs: []string{"list", "watch"}, - }, - }, - }, - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "", - }, - ), - ) - - require.Equal(t, v1alpha1.ClusterServiceVersion{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterServiceVersion", - APIVersion: v1alpha1.SchemeGroupVersion.String(), - }, - Spec: v1alpha1.ClusterServiceVersionSpec{ - InstallStrategy: v1alpha1.NamedInstallStrategy{ - StrategySpec: v1alpha1.StrategyDetailsDeployment{ - ClusterPermissions: []v1alpha1.StrategyDeploymentPermissions{ - { - ServiceAccountName: "service-account", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"secrets"}, - Verbs: []string{"list", "watch"}, - }, - }, - }, - { - ServiceAccountName: "", - }, - }, - }, - }, - }, - }, csv) -} - -func Test_MakeCSV_WithOwnedCRDs(t *testing.T) { - csv := MakeCSV( - WithOwnedCRDs( - v1alpha1.CRDDescription{Name: "a.crd.something"}, - v1alpha1.CRDDescription{Name: "b.crd.something"}, - ), - ) - - require.Equal(t, v1alpha1.ClusterServiceVersion{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterServiceVersion", - APIVersion: v1alpha1.SchemeGroupVersion.String(), - }, - Spec: v1alpha1.ClusterServiceVersionSpec{ - CustomResourceDefinitions: v1alpha1.CustomResourceDefinitions{ - Owned: []v1alpha1.CRDDescription{ - {Name: "a.crd.something"}, - {Name: "b.crd.something"}, - }, - }, - }, - }, csv) -} diff --git a/internal/operator-controller/rukpak/util/util.go b/internal/operator-controller/rukpak/util/util.go index b6f64d20b..de96c9c0c 100644 --- a/internal/operator-controller/rukpak/util/util.go +++ b/internal/operator-controller/rukpak/util/util.go @@ -4,6 +4,8 @@ import ( "fmt" "io" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/cli-runtime/pkg/resource" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -17,6 +19,21 @@ func ObjectNameForBaseAndSuffix(base string, suffix string) string { return fmt.Sprintf("%s-%s", base, suffix) } +func ToUnstructured(obj client.Object) (*unstructured.Unstructured, error) { + gvk := obj.GetObjectKind().GroupVersionKind() + + var u unstructured.Unstructured + uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + if err != nil { + return nil, fmt.Errorf("convert %s %q to unstructured: %w", gvk.Kind, obj.GetName(), err) + } + unstructured.RemoveNestedField(uObj, "metadata", "creationTimestamp") + unstructured.RemoveNestedField(uObj, "status") + u.Object = uObj + u.SetGroupVersionKind(gvk) + return &u, nil +} + func MergeMaps(maps ...map[string]string) map[string]string { out := map[string]string{} for _, m := range maps { From 96c6e2d221f4326ef7152616a4f37cdad468cd90 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Tue, 29 Apr 2025 13:21:30 +0200 Subject: [PATCH 03/11] Update webhook configuration naming Signed-off-by: Per Goncalves da Silva --- .../rukpak/render/generators/generators.go | 10 +- .../render/generators/generators_test.go | 210 +++++++++++++++++- .../rukpak/render/generators/resources.go | 19 +- .../render/generators/resources_test.go | 4 +- 4 files changed, 218 insertions(+), 25 deletions(-) diff --git a/internal/operator-controller/rukpak/render/generators/generators.go b/internal/operator-controller/rukpak/render/generators/generators.go index 31f8857cd..5e702c492 100644 --- a/internal/operator-controller/rukpak/render/generators/generators.go +++ b/internal/operator-controller/rukpak/render/generators/generators.go @@ -304,12 +304,13 @@ func BundleValidatingWebhookResourceGenerator(rv1 *render.RegistryV1, opts rende continue } certProvisioner := render.CertProvisionerFor(wh.DeploymentName, opts) + webhookName := strings.TrimSuffix(wh.GenerateName, "-") webhookResource := CreateValidatingWebhookConfigurationResource( - wh.GenerateName, + webhookName, opts.InstallNamespace, WithValidatingWebhooks( admissionregistrationv1.ValidatingWebhook{ - Name: wh.GenerateName, + Name: webhookName, Rules: wh.Rules, FailurePolicy: wh.FailurePolicy, MatchPolicy: wh.MatchPolicy, @@ -351,12 +352,13 @@ func BundleMutatingWebhookResourceGenerator(rv1 *render.RegistryV1, opts render. continue } certProvisioner := render.CertProvisionerFor(wh.DeploymentName, opts) + webhookName := strings.TrimSuffix(wh.GenerateName, "-") webhookResource := CreateMutatingWebhookConfigurationResource( - wh.GenerateName, + webhookName, opts.InstallNamespace, WithMutatingWebhooks( admissionregistrationv1.MutatingWebhook{ - Name: wh.GenerateName, + Name: webhookName, Rules: wh.Rules, FailurePolicy: wh.FailurePolicy, MatchPolicy: wh.MatchPolicy, diff --git a/internal/operator-controller/rukpak/render/generators/generators_test.go b/internal/operator-controller/rukpak/render/generators/generators_test.go index d47c84abb..0dcb9b11e 100644 --- a/internal/operator-controller/rukpak/render/generators/generators_test.go +++ b/internal/operator-controller/rukpak/render/generators/generators_test.go @@ -1547,8 +1547,101 @@ func Test_BundleValidatingWebhookResourceGenerator_Succeeds(t *testing.T) { APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ - GenerateName: "my-webhook-", - Namespace: "install-namespace", + Name: "my-webhook", + Namespace: "install-namespace", + }, + Webhooks: []admissionregistrationv1.ValidatingWebhook{ + { + Name: "my-webhook", + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.OperationAll, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{""}, + Resources: []string{"namespaces"}, + }, + }, + }, + FailurePolicy: ptr.To(admissionregistrationv1.Fail), + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + SideEffects: ptr.To(admissionregistrationv1.SideEffectClassNone), + TimeoutSeconds: ptr.To(int32(1)), + AdmissionReviewVersions: []string{ + "v1beta1", + "v1beta2", + }, + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: "install-namespace", + Name: "my-deployment-service", + Path: ptr.To("/webhook-path"), + Port: ptr.To(int32(443)), + }, + }, + }, + }, + }, + }, + }, + { + name: "removes any - suffixes from the webhook name (v0 used GenerateName to allow multiple operator installations - we don't want that in v1)", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "my-webhook-", + DeploymentName: "my-deployment", + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.OperationAll, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{""}, + Resources: []string{"namespaces"}, + }, + }, + }, + FailurePolicy: ptr.To(admissionregistrationv1.Fail), + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + SideEffects: ptr.To(admissionregistrationv1.SideEffectClassNone), + TimeoutSeconds: ptr.To(int32(1)), + AdmissionReviewVersions: []string{ + "v1beta1", + "v1beta2", + }, + WebhookPath: ptr.To("/webhook-path"), + ContainerPort: 443, + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &admissionregistrationv1.ValidatingWebhookConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "ValidatingWebhookConfiguration", + APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-webhook", + Namespace: "install-namespace", }, Webhooks: []admissionregistrationv1.ValidatingWebhook{ { @@ -1616,8 +1709,8 @@ func Test_BundleValidatingWebhookResourceGenerator_Succeeds(t *testing.T) { APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ - GenerateName: "my-webhook-", - Namespace: "install-namespace", + Name: "my-webhook", + Namespace: "install-namespace", Annotations: map[string]string{ "cert-provider": "annotation", }, @@ -1719,8 +1812,103 @@ func Test_BundleMutatingWebhookResourceGenerator_Succeeds(t *testing.T) { APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ - GenerateName: "my-webhook-", - Namespace: "install-namespace", + Name: "my-webhook", + Namespace: "install-namespace", + }, + Webhooks: []admissionregistrationv1.MutatingWebhook{ + { + Name: "my-webhook", + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.OperationAll, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{""}, + Resources: []string{"namespaces"}, + }, + }, + }, + FailurePolicy: ptr.To(admissionregistrationv1.Fail), + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + SideEffects: ptr.To(admissionregistrationv1.SideEffectClassNone), + TimeoutSeconds: ptr.To(int32(1)), + AdmissionReviewVersions: []string{ + "v1beta1", + "v1beta2", + }, + ReinvocationPolicy: ptr.To(admissionregistrationv1.IfNeededReinvocationPolicy), + ClientConfig: admissionregistrationv1.WebhookClientConfig{ + Service: &admissionregistrationv1.ServiceReference{ + Namespace: "install-namespace", + Name: "my-deployment-service", + Path: ptr.To("/webhook-path"), + Port: ptr.To(int32(443)), + }, + }, + }, + }, + }, + }, + }, + { + name: "removes any - suffixes from the webhook name (v0 used GenerateName to allow multiple operator installations - we don't want that in v1)", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "my-webhook-", + DeploymentName: "my-deployment", + Rules: []admissionregistrationv1.RuleWithOperations{ + { + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.OperationAll, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{""}, + Resources: []string{"namespaces"}, + }, + }, + }, + FailurePolicy: ptr.To(admissionregistrationv1.Fail), + ObjectSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "foo": "bar", + }, + }, + SideEffects: ptr.To(admissionregistrationv1.SideEffectClassNone), + TimeoutSeconds: ptr.To(int32(1)), + AdmissionReviewVersions: []string{ + "v1beta1", + "v1beta2", + }, + WebhookPath: ptr.To("/webhook-path"), + ContainerPort: 443, + ReinvocationPolicy: ptr.To(admissionregistrationv1.IfNeededReinvocationPolicy), + }, + ), + ), + }, + opts: render.Options{ + InstallNamespace: "install-namespace", + TargetNamespaces: []string{"watch-namespace-one", "watch-namespace-two"}, + }, + expectedResources: []client.Object{ + &admissionregistrationv1.MutatingWebhookConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "MutatingWebhookConfiguration", + APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-webhook", + Namespace: "install-namespace", }, Webhooks: []admissionregistrationv1.MutatingWebhook{ { @@ -1789,8 +1977,8 @@ func Test_BundleMutatingWebhookResourceGenerator_Succeeds(t *testing.T) { APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ - GenerateName: "my-webhook-", - Namespace: "install-namespace", + Name: "my-webhook", + Namespace: "install-namespace", Annotations: map[string]string{ "cert-provider": "annotation", }, @@ -2258,6 +2446,7 @@ func Test_CertProviderResourceGenerator_Succeeds(t *testing.T) { fakeProvider := FakeCertProvider{ AdditionalObjectsFn: func(cfg render.CertificateProvisionerConfig) ([]unstructured.Unstructured, error) { return []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{ + TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()}, ObjectMeta: metav1.ObjectMeta{ Name: cfg.CertName, }, @@ -2290,9 +2479,8 @@ func Test_CertProviderResourceGenerator_Succeeds(t *testing.T) { require.NoError(t, err) require.Equal(t, []client.Object{ ToUnstructuredT(t, &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "my-deployment-service-cert", - }, + TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Name: "my-deployment-service-cert"}, }), }, objs) } diff --git a/internal/operator-controller/rukpak/render/generators/resources.go b/internal/operator-controller/rukpak/render/generators/resources.go index ddcfd3e15..ed1cf6552 100644 --- a/internal/operator-controller/rukpak/render/generators/resources.go +++ b/internal/operator-controller/rukpak/render/generators/resources.go @@ -1,8 +1,6 @@ package generators import ( - "fmt" - admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -217,7 +215,9 @@ func CreateDeploymentResource(name string, namespace string, opts ...ResourceCre ).(*appsv1.Deployment) } -func CreateValidatingWebhookConfigurationResource(generateName string, namespace string, opts ...ResourceCreatorOption) *admissionregistrationv1.ValidatingWebhookConfiguration { +// CreateValidatingWebhookConfigurationResource creates a ValidatingWebhookConfiguration resource with name 'name', +// namespace 'namespace', and applying any ValidatingWebhookConfiguration related options in opts +func CreateValidatingWebhookConfigurationResource(name string, namespace string, opts ...ResourceCreatorOption) *admissionregistrationv1.ValidatingWebhookConfiguration { return ResourceCreatorOptions(opts).ApplyTo( &admissionregistrationv1.ValidatingWebhookConfiguration{ TypeMeta: metav1.TypeMeta{ @@ -225,14 +225,16 @@ func CreateValidatingWebhookConfigurationResource(generateName string, namespace APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("%s-", generateName), - Namespace: namespace, + Name: name, + Namespace: namespace, }, }, ).(*admissionregistrationv1.ValidatingWebhookConfiguration) } -func CreateMutatingWebhookConfigurationResource(generateName string, namespace string, opts ...ResourceCreatorOption) *admissionregistrationv1.MutatingWebhookConfiguration { +// CreateMutatingWebhookConfigurationResource creates a MutatingWebhookConfiguration resource with name 'name', +// namespace 'namespace', and applying any MutatingWebhookConfiguration related options in opts +func CreateMutatingWebhookConfigurationResource(name string, namespace string, opts ...ResourceCreatorOption) *admissionregistrationv1.MutatingWebhookConfiguration { return ResourceCreatorOptions(opts).ApplyTo( &admissionregistrationv1.MutatingWebhookConfiguration{ TypeMeta: metav1.TypeMeta{ @@ -240,13 +242,14 @@ func CreateMutatingWebhookConfigurationResource(generateName string, namespace s APIVersion: admissionregistrationv1.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ - GenerateName: fmt.Sprintf("%s-", generateName), - Namespace: namespace, + Name: name, + Namespace: namespace, }, }, ).(*admissionregistrationv1.MutatingWebhookConfiguration) } +// CreateServiceResource creates a Service resource with name 'name', namespace 'namespace', and applying any Service related options in opts func CreateServiceResource(name string, namespace string, opts ...ResourceCreatorOption) *corev1.Service { return ResourceCreatorOptions(opts).ApplyTo(&corev1.Service{ TypeMeta: metav1.TypeMeta{ diff --git a/internal/operator-controller/rukpak/render/generators/resources_test.go b/internal/operator-controller/rukpak/render/generators/resources_test.go index 1eef933e5..7d6a95e33 100644 --- a/internal/operator-controller/rukpak/render/generators/resources_test.go +++ b/internal/operator-controller/rukpak/render/generators/resources_test.go @@ -85,14 +85,14 @@ func Test_CreateService(t *testing.T) { func Test_CreateValidatingWebhookConfiguration(t *testing.T) { wh := generators.CreateValidatingWebhookConfigurationResource("my-validating-webhook-configuration", "my-namespace") require.NotNil(t, wh) - require.Equal(t, "my-validating-webhook-configuration-", wh.GenerateName) + require.Equal(t, "my-validating-webhook-configuration", wh.Name) require.Equal(t, "my-namespace", wh.Namespace) } func Test_CreateMutatingWebhookConfiguration(t *testing.T) { wh := generators.CreateMutatingWebhookConfigurationResource("my-mutating-webhook-configuration", "my-namespace") require.NotNil(t, wh) - require.Equal(t, "my-mutating-webhook-configuration-", wh.GenerateName) + require.Equal(t, "my-mutating-webhook-configuration", wh.Name) require.Equal(t, "my-namespace", wh.Namespace) } From 9050a4f4066348371e839addbbc447ab7da0cf0a Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 30 Apr 2025 10:06:48 +0200 Subject: [PATCH 04/11] Harden ToUnstructured and add unit tests Signed-off-by: Per Goncalves da Silva --- .../rukpak/render/certprovider_test.go | 9 ++- .../operator-controller/rukpak/util/util.go | 13 ++++ .../rukpak/util/util_test.go | 60 ++++++++++++++++--- 3 files changed, 71 insertions(+), 11 deletions(-) diff --git a/internal/operator-controller/rukpak/render/certprovider_test.go b/internal/operator-controller/rukpak/render/certprovider_test.go index 293a86b46..3005cfd73 100644 --- a/internal/operator-controller/rukpak/render/certprovider_test.go +++ b/internal/operator-controller/rukpak/render/certprovider_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client" @@ -36,7 +37,9 @@ func Test_CertificateProvisioner_WithCertProvider(t *testing.T) { return nil }, AdditionalObjectsFn: func(cfg render.CertificateProvisionerConfig) ([]unstructured.Unstructured, error) { - return []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{})}, nil + return []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{ + TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()}, + })}, nil }, GetCertSecretInfoFn: func(cfg render.CertificateProvisionerConfig) render.CertSecretInfo { return render.CertSecretInfo{ @@ -59,7 +62,9 @@ func Test_CertificateProvisioner_WithCertProvider(t *testing.T) { objs, err := provisioner.AdditionalObjects() require.NoError(t, err) - require.Equal(t, []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{})}, objs) + require.Equal(t, []unstructured.Unstructured{*ToUnstructuredT(t, &corev1.Secret{ + TypeMeta: metav1.TypeMeta{Kind: "Secret", APIVersion: corev1.SchemeGroupVersion.String()}, + })}, objs) require.Equal(t, &render.CertSecretInfo{ SecretName: "some-secret", diff --git a/internal/operator-controller/rukpak/util/util.go b/internal/operator-controller/rukpak/util/util.go index de96c9c0c..503d7afa7 100644 --- a/internal/operator-controller/rukpak/util/util.go +++ b/internal/operator-controller/rukpak/util/util.go @@ -1,6 +1,7 @@ package util import ( + "errors" "fmt" "io" @@ -19,8 +20,20 @@ func ObjectNameForBaseAndSuffix(base string, suffix string) string { return fmt.Sprintf("%s-%s", base, suffix) } +// ToUnstructured converts obj into an Unstructured. It expects the obj's gvk to be defined. If it is not, +// an error will be returned. func ToUnstructured(obj client.Object) (*unstructured.Unstructured, error) { + if obj == nil { + return nil, errors.New("object is nil") + } + gvk := obj.GetObjectKind().GroupVersionKind() + if len(gvk.Kind) == 0 { + return nil, errors.New("object has no kind") + } + if len(gvk.Version) == 0 { + return nil, errors.New("object has no version") + } var u unstructured.Unstructured uObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) diff --git a/internal/operator-controller/rukpak/util/util_test.go b/internal/operator-controller/rukpak/util/util_test.go index f5048abf1..60c1cd646 100644 --- a/internal/operator-controller/rukpak/util/util_test.go +++ b/internal/operator-controller/rukpak/util/util_test.go @@ -56,15 +56,6 @@ func TestMergeMaps(t *testing.T) { } } -// Mock reader for testing that always returns an error when Read is called -type errorReader struct { - io.Reader -} - -func (m errorReader) Read(p []byte) (int, error) { - return 0, errors.New("Oh no!") -} - func TestManifestObjects(t *testing.T) { tests := []struct { name string @@ -152,3 +143,54 @@ spec: }) } } + +func Test_ToUnstructured(t *testing.T) { + for _, tc := range []struct { + name string + obj client.Object + err error + }{ + { + name: "converts object to unstructured", + obj: &corev1.Service{ + TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "my-service", Namespace: "my-namespace"}, + }, + }, { + name: "fails if object doesn't define kind", + obj: &corev1.Service{ + TypeMeta: metav1.TypeMeta{Kind: "", APIVersion: "v1"}, + ObjectMeta: metav1.ObjectMeta{Name: "my-service", Namespace: "my-namespace"}, + }, + err: errors.New("object has no kind"), + }, { + name: "fails if object doesn't define version", + obj: &corev1.Service{ + TypeMeta: metav1.TypeMeta{Kind: "Service", APIVersion: ""}, + ObjectMeta: metav1.ObjectMeta{Name: "my-service", Namespace: "my-namespace"}, + }, + err: errors.New("object has no version"), + }, { + name: "fails if object is nil", + err: errors.New("object is nil"), + }, + } { + t.Run(tc.name, func(t *testing.T) { + out, err := util.ToUnstructured(tc.obj) + if tc.err != nil { + require.Error(t, err) + } else { + assert.Equal(t, tc.obj.GetObjectKind().GroupVersionKind(), out.GroupVersionKind()) + } + }) + } +} + +// Mock reader for testing that always returns an error when Read is called +type errorReader struct { + io.Reader +} + +func (m errorReader) Read(p []byte) (int, error) { + return 0, errors.New("Oh no!") +} From 82e225dc3f37535da121b41ae51661c8aceb2afd Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Wed, 30 Apr 2025 10:32:29 +0200 Subject: [PATCH 05/11] Move cert provider option to render Signed-off-by: Per Goncalves da Silva --- internal/operator-controller/rukpak/convert/registryv1.go | 2 +- .../rukpak/render/certproviders/certmanager.go | 6 ------ internal/operator-controller/rukpak/render/render.go | 6 ++++++ internal/operator-controller/rukpak/render/render_test.go | 8 ++++++++ 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/internal/operator-controller/rukpak/convert/registryv1.go b/internal/operator-controller/rukpak/convert/registryv1.go index fcb6a1d5c..aa17a08e2 100644 --- a/internal/operator-controller/rukpak/convert/registryv1.go +++ b/internal/operator-controller/rukpak/convert/registryv1.go @@ -271,7 +271,7 @@ func (c Converter) Convert(rv1 render.RegistryV1, installNamespace string, targe rv1, installNamespace, render.WithTargetNamespaces(targetNamespaces...), - certproviders.WithCertManagerCertificateProvider(), + render.WithCertificateProvider(certproviders.CertManagerCertificateProvider{}), ) if err != nil { return nil, err diff --git a/internal/operator-controller/rukpak/render/certproviders/certmanager.go b/internal/operator-controller/rukpak/render/certproviders/certmanager.go index b2d32d7bd..f8b7d06a1 100644 --- a/internal/operator-controller/rukpak/render/certproviders/certmanager.go +++ b/internal/operator-controller/rukpak/render/certproviders/certmanager.go @@ -20,12 +20,6 @@ const ( certManagerInjectCAAnnotation = "cert-manager.io/inject-ca-from" ) -func WithCertManagerCertificateProvider() render.Option { - return func(o *render.Options) { - o.CertificateProvider = CertManagerCertificateProvider{} - } -} - var _ render.CertificateProvider = (*CertManagerCertificateProvider)(nil) type CertManagerCertificateProvider struct{} diff --git a/internal/operator-controller/rukpak/render/render.go b/internal/operator-controller/rukpak/render/render.go index 03922e454..7e7a599bf 100644 --- a/internal/operator-controller/rukpak/render/render.go +++ b/internal/operator-controller/rukpak/render/render.go @@ -92,6 +92,12 @@ func WithUniqueNameGenerator(generator UniqueNameGenerator) Option { } } +func WithCertificateProvider(provider CertificateProvider) Option { + return func(o *Options) { + o.CertificateProvider = provider + } +} + type BundleRenderer struct { BundleValidator BundleValidator ResourceGenerators []ResourceGenerator diff --git a/internal/operator-controller/rukpak/render/render_test.go b/internal/operator-controller/rukpak/render/render_test.go index 510a62987..07409ed5b 100644 --- a/internal/operator-controller/rukpak/render/render_test.go +++ b/internal/operator-controller/rukpak/render/render_test.go @@ -12,6 +12,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" + . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" ) func Test_BundleRenderer_NoConfig(t *testing.T) { @@ -81,6 +82,13 @@ func Test_WithUniqueNameGenerator(t *testing.T) { require.Equal(t, "a man needs a name", generatedName) } +func Test_WithCertificateProvide(t *testing.T) { + opts := &render.Options{} + expectedCertProvider := FakeCertProvider{} + render.WithCertificateProvider(expectedCertProvider)(opts) + require.Equal(t, expectedCertProvider, opts.CertificateProvider) +} + func Test_BundleRenderer_CallsResourceGenerators(t *testing.T) { renderer := render.BundleRenderer{ ResourceGenerators: []render.ResourceGenerator{ From 9c485fa7118435bfafd1cbe13452adeb0fee364d Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Mon, 5 May 2025 09:42:41 +0200 Subject: [PATCH 06/11] Add resource name validation Signed-off-by: Per Goncalves da Silva --- .../rukpak/render/validators/validator.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/internal/operator-controller/rukpak/render/validators/validator.go b/internal/operator-controller/rukpak/render/validators/validator.go index ee77292a3..42cca6a03 100644 --- a/internal/operator-controller/rukpak/render/validators/validator.go +++ b/internal/operator-controller/rukpak/render/validators/validator.go @@ -4,6 +4,7 @@ import ( "cmp" "errors" "fmt" + "k8s.io/apimachinery/pkg/util/validation" "maps" "slices" "strings" @@ -49,6 +50,25 @@ func CheckDeploymentSpecUniqueness(rv1 *render.RegistryV1) []error { return errs } +// CheckDeploymentNameIsDNS1123SubDomain checks each deployment strategy spec name complies with the Kubernetes +// resource naming conversions +func CheckDeploymentNameIsDNS1123SubDomain(rv1 *render.RegistryV1) []error { + deploymentNameErrMap := map[string][]string{} + for _, dep := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { + errs := validation.IsDNS1123Subdomain(dep.Name) + if len(errs) > 0 { + slices.Sort(errs) + deploymentNameErrMap[dep.Name] = errs + } + } + + var errs []error + for _, dep := range slices.Sorted(maps.Keys(deploymentNameErrMap)) { + errs = append(errs, fmt.Errorf("invalid cluster service version strategy deployment name '%s': %s", dep, strings.Join(deploymentNameErrMap[dep], ", "))) + } + return errs +} + // CheckOwnedCRDExistence checks bundle owned custom resource definitions declared in the csv exist in the bundle func CheckOwnedCRDExistence(rv1 *render.RegistryV1) []error { crdsNames := sets.Set[string]{} From 08748d47602b25cdf43bc7d94b464c3fa0f6f950 Mon Sep 17 00:00:00 2001 From: Per Goncalves da Silva Date: Mon, 5 May 2025 12:09:36 +0200 Subject: [PATCH 07/11] Add name validation for strategy deployment and webhook configurations Signed-off-by: Per Goncalves da Silva --- .../rukpak/convert/registryv1_test.go | 17 +-- .../rukpak/render/validators/validator.go | 31 ++++- .../render/validators/validator_test.go | 121 +++++++++++++++++- 3 files changed, 151 insertions(+), 18 deletions(-) diff --git a/internal/operator-controller/rukpak/convert/registryv1_test.go b/internal/operator-controller/rukpak/convert/registryv1_test.go index 49296ae56..29a7f6217 100644 --- a/internal/operator-controller/rukpak/convert/registryv1_test.go +++ b/internal/operator-controller/rukpak/convert/registryv1_test.go @@ -206,7 +206,7 @@ func getBaseCsvAndService() (v1alpha1.ClusterServiceVersion, corev1.Service) { }), WithStrategyDeploymentSpecs( v1alpha1.StrategyDeploymentSpec{ - Name: "testDeployment", + Name: "test-deployment", Spec: appsv1.DeploymentSpec{ Template: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ @@ -271,7 +271,7 @@ func TestRegistryV1SuiteGenerateAllNamespace(t *testing.T) { require.Len(t, plainBundle.Objects, 5) t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template") - dep := findObjectByName("testDeployment", plainBundle.Objects) + dep := findObjectByName("test-deployment", plainBundle.Objects) require.NotNil(t, dep) require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces) require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces]) @@ -304,7 +304,7 @@ func TestRegistryV1SuiteGenerateMultiNamespace(t *testing.T) { require.Len(t, plainBundle.Objects, 7) t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template") - dep := findObjectByName("testDeployment", plainBundle.Objects) + dep := findObjectByName("test-deployment", plainBundle.Objects) require.NotNil(t, dep) require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces) require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces]) @@ -337,7 +337,7 @@ func TestRegistryV1SuiteGenerateSingleNamespace(t *testing.T) { require.Len(t, plainBundle.Objects, 5) t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template") - dep := findObjectByName("testDeployment", plainBundle.Objects) + dep := findObjectByName("test-deployment", plainBundle.Objects) require.NotNil(t, dep) require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces) require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces]) @@ -370,7 +370,7 @@ func TestRegistryV1SuiteGenerateOwnNamespace(t *testing.T) { require.Len(t, plainBundle.Objects, 5) t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template") - dep := findObjectByName("testDeployment", plainBundle.Objects) + dep := findObjectByName("test-deployment", plainBundle.Objects) require.NotNil(t, dep) require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces) require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces]) @@ -575,7 +575,7 @@ func TestRegistryV1SuiteGenerateWebhooks_WebhookSupportFGEnabled(t *testing.T) { CRDs: []apiextensionsv1.CustomResourceDefinition{ { ObjectMeta: metav1.ObjectMeta{ - Name: "fake-webhook.package-with-webhooks.io", + Name: "fake-webhook.package-with-webhooks", }, }, }, @@ -584,7 +584,7 @@ func TestRegistryV1SuiteGenerateWebhooks_WebhookSupportFGEnabled(t *testing.T) { WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), WithOwnedCRDs( v1alpha1.CRDDescription{ - Name: "fake-webhook.package-with-webhooks.io", + Name: "fake-webhook.package-with-webhooks", }, ), WithStrategyDeploymentSpecs( @@ -595,8 +595,9 @@ func TestRegistryV1SuiteGenerateWebhooks_WebhookSupportFGEnabled(t *testing.T) { WithWebhookDefinitions( v1alpha1.WebhookDescription{ Type: v1alpha1.ConversionWebhook, - ConversionCRDs: []string{"fake-webhook.package-with-webhooks.io"}, + ConversionCRDs: []string{"fake-webhook.package-with-webhooks"}, DeploymentName: "some-deployment", + GenerateName: "my-conversion-webhook", }, ), ), diff --git a/internal/operator-controller/rukpak/render/validators/validator.go b/internal/operator-controller/rukpak/render/validators/validator.go index 42cca6a03..4d9568375 100644 --- a/internal/operator-controller/rukpak/render/validators/validator.go +++ b/internal/operator-controller/rukpak/render/validators/validator.go @@ -4,12 +4,12 @@ import ( "cmp" "errors" "fmt" - "k8s.io/apimachinery/pkg/util/validation" "maps" "slices" "strings" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" "github.com/operator-framework/api/pkg/operators/v1alpha1" @@ -22,11 +22,13 @@ var RegistryV1BundleValidator = render.BundleValidator{ // you bring the same changes over to that test. This helps ensure all validation rules are executed // while giving us the flexibility to test each validation function individually CheckDeploymentSpecUniqueness, + CheckDeploymentNameIsDNS1123SubDomain, CheckCRDResourceUniqueness, CheckOwnedCRDExistence, CheckPackageNameNotEmpty, CheckWebhookDeploymentReferentialIntegrity, CheckWebhookNameUniqueness, + CheckWebhookNameIsDNS1123SubDomain, CheckConversionWebhookCRDReferenceUniqueness, CheckConversionWebhooksReferenceOwnedCRDs, } @@ -62,7 +64,7 @@ func CheckDeploymentNameIsDNS1123SubDomain(rv1 *render.RegistryV1) []error { } } - var errs []error + errs := make([]error, 0, len(deploymentNameErrMap)) for _, dep := range slices.Sorted(maps.Keys(deploymentNameErrMap)) { errs = append(errs, fmt.Errorf("invalid cluster service version strategy deployment name '%s': %s", dep, strings.Join(deploymentNameErrMap[dep], ", "))) } @@ -158,7 +160,7 @@ func CheckWebhookDeploymentReferentialIntegrity(rv1 *render.RegistryV1) []error return cmp.Or(cmp.Compare(a.Type, b.Type), cmp.Compare(a.GenerateName, b.GenerateName)) }) for _, webhookDef := range webhookDefns { - errs = append(errs, fmt.Errorf("webhook '%s' of type '%s' references non-existent deployment '%s'", webhookDef.GenerateName, webhookDef.Type, webhookDef.DeploymentName)) + errs = append(errs, fmt.Errorf("webhook of type '%s' with name '%s' references non-existent deployment '%s'", webhookDef.Type, webhookDef.GenerateName, webhookDef.DeploymentName)) } } return errs @@ -256,3 +258,26 @@ func CheckConversionWebhookCRDReferenceUniqueness(rv1 *render.RegistryV1) []erro } return errs } + +// CheckWebhookNameIsDNS1123SubDomain checks each webhook configuration name complies with the Kubernetes resource naming conversions +func CheckWebhookNameIsDNS1123SubDomain(rv1 *render.RegistryV1) []error { + invalidWebhooksByType := map[v1alpha1.WebhookAdmissionType]map[string][]string{} + for _, wh := range rv1.CSV.Spec.WebhookDefinitions { + if _, ok := invalidWebhooksByType[wh.Type]; !ok { + invalidWebhooksByType[wh.Type] = map[string][]string{} + } + errs := validation.IsDNS1123Subdomain(wh.GenerateName) + if len(errs) > 0 { + slices.Sort(errs) + invalidWebhooksByType[wh.Type][wh.GenerateName] = errs + } + } + + var errs []error + for _, whType := range slices.Sorted(maps.Keys(invalidWebhooksByType)) { + for _, webhookName := range slices.Sorted(maps.Keys(invalidWebhooksByType[whType])) { + errs = append(errs, fmt.Errorf("webhook of type '%s' has invalid name '%s': %s", whType, webhookName, strings.Join(invalidWebhooksByType[whType][webhookName], ","))) + } + } + return errs +} diff --git a/internal/operator-controller/rukpak/render/validators/validator_test.go b/internal/operator-controller/rukpak/render/validators/validator_test.go index d8a480bf6..b6feae4bd 100644 --- a/internal/operator-controller/rukpak/render/validators/validator_test.go +++ b/internal/operator-controller/rukpak/render/validators/validator_test.go @@ -19,11 +19,13 @@ import ( func Test_BundleValidatorHasAllValidationFns(t *testing.T) { expectedValidationFns := []func(v1 *render.RegistryV1) []error{ validators.CheckDeploymentSpecUniqueness, + validators.CheckDeploymentNameIsDNS1123SubDomain, validators.CheckCRDResourceUniqueness, validators.CheckOwnedCRDExistence, validators.CheckPackageNameNotEmpty, validators.CheckWebhookDeploymentReferentialIntegrity, validators.CheckWebhookNameUniqueness, + validators.CheckWebhookNameIsDNS1123SubDomain, validators.CheckConversionWebhookCRDReferenceUniqueness, validators.CheckConversionWebhooksReferenceOwnedCRDs, } @@ -92,6 +94,48 @@ func Test_CheckDeploymentSpecUniqueness(t *testing.T) { } } +func Test_CheckDeploymentNameIsDNS1123SubDomain(t *testing.T) { + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + expectedErrs []error + }{ + { + name: "accepts valid deployment strategy spec names", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-one"}, + v1alpha1.StrategyDeploymentSpec{Name: "test-deployment-two"}, + ), + ), + }, + expectedErrs: []error{}, + }, { + name: "rejects bundles with invalid deployment strategy spec names - errors are sorted by name", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithStrategyDeploymentSpecs( + v1alpha1.StrategyDeploymentSpec{Name: "-bad-name"}, + v1alpha1.StrategyDeploymentSpec{Name: "b-name-is-waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay-too-long"}, + v1alpha1.StrategyDeploymentSpec{Name: "a-name-is-waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay-too-long-and-bad-"}, + ), + ), + }, + expectedErrs: []error{ + errors.New("invalid cluster service version strategy deployment name '-bad-name': a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')"), + errors.New("invalid cluster service version strategy deployment name 'a-name-is-waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay-too-long-and-bad-': a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'), must be no more than 253 characters"), + errors.New("invalid cluster service version strategy deployment name 'b-name-is-waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay-too-long': must be no more than 253 characters"), + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := validators.CheckDeploymentNameIsDNS1123SubDomain(tc.bundle) + require.Equal(t, tc.expectedErrs, errs) + }) + } +} + func Test_CRDResourceUniqueness(t *testing.T) { for _, tc := range []struct { name string @@ -356,7 +400,7 @@ func Test_CheckWebhookDeploymentReferentialIntegrity(t *testing.T) { ), }, expectedErrs: []error{ - errors.New("webhook 'test-webhook' of type 'ValidatingAdmissionWebhook' references non-existent deployment 'test-deployment-two'"), + errors.New("webhook of type 'ValidatingAdmissionWebhook' with name 'test-webhook' references non-existent deployment 'test-deployment-two'"), }, }, { name: "errors are ordered by deployment strategy spec name, webhook type, and webhook name", @@ -398,12 +442,12 @@ func Test_CheckWebhookDeploymentReferentialIntegrity(t *testing.T) { ), }, expectedErrs: []error{ - errors.New("webhook 'test-mute-webhook-a' of type 'MutatingAdmissionWebhook' references non-existent deployment 'test-deployment-a'"), - errors.New("webhook 'test-conv-webhook-b' of type 'ConversionWebhook' references non-existent deployment 'test-deployment-b'"), - errors.New("webhook 'test-conv-webhook-c-a' of type 'ConversionWebhook' references non-existent deployment 'test-deployment-c'"), - errors.New("webhook 'test-conv-webhook-c-b' of type 'ConversionWebhook' references non-existent deployment 'test-deployment-c'"), - errors.New("webhook 'test-mute-webhook-c' of type 'MutatingAdmissionWebhook' references non-existent deployment 'test-deployment-c'"), - errors.New("webhook 'test-val-webhook-c' of type 'ValidatingAdmissionWebhook' references non-existent deployment 'test-deployment-c'"), + errors.New("webhook of type 'MutatingAdmissionWebhook' with name 'test-mute-webhook-a' references non-existent deployment 'test-deployment-a'"), + errors.New("webhook of type 'ConversionWebhook' with name 'test-conv-webhook-b' references non-existent deployment 'test-deployment-b'"), + errors.New("webhook of type 'ConversionWebhook' with name 'test-conv-webhook-c-a' references non-existent deployment 'test-deployment-c'"), + errors.New("webhook of type 'ConversionWebhook' with name 'test-conv-webhook-c-b' references non-existent deployment 'test-deployment-c'"), + errors.New("webhook of type 'MutatingAdmissionWebhook' with name 'test-mute-webhook-c' references non-existent deployment 'test-deployment-c'"), + errors.New("webhook of type 'ValidatingAdmissionWebhook' with name 'test-val-webhook-c' references non-existent deployment 'test-deployment-c'"), }, }, } { @@ -837,3 +881,66 @@ func Test_CheckConversionWebhookCRDReferenceUniqueness(t *testing.T) { }) } } + +func Test_CheckWebhookNameIsDNS1123SubDomain(t *testing.T) { + for _, tc := range []struct { + name string + bundle *render.RegistryV1 + expectedErrs []error + }{ + { + name: "accepts bundles without webhook definitions", + bundle: &render.RegistryV1{ + CSV: MakeCSV(), + }, + }, { + name: "rejects bundles with invalid webhook definitions names and orders errors by webhook type and name", + bundle: &render.RegistryV1{ + CSV: MakeCSV( + WithWebhookDefinitions( + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "b-name-is-waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay-too-long-and-bad-", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "a-name-is-waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay-too-long", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ValidatingAdmissionWebhook, + GenerateName: "-bad-name", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "b-bad-name-", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "b-name-is-waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay-too-long-and-bad-", + }, v1alpha1.WebhookDescription{ + Type: v1alpha1.MutatingAdmissionWebhook, + GenerateName: "a-bad-name-", + }, + v1alpha1.WebhookDescription{ + Type: v1alpha1.ConversionWebhook, + GenerateName: "a-bad-name-", + }, + ), + ), + }, + expectedErrs: []error{ + errors.New("webhook of type 'ConversionWebhook' has invalid name 'a-bad-name-': a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')"), + errors.New("webhook of type 'ConversionWebhook' has invalid name 'b-bad-name-': a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')"), + errors.New("webhook of type 'MutatingAdmissionWebhook' has invalid name 'a-bad-name-': a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')"), + errors.New("webhook of type 'MutatingAdmissionWebhook' has invalid name 'b-name-is-waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay-too-long-and-bad-': a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'),must be no more than 253 characters"), + errors.New("webhook of type 'ValidatingAdmissionWebhook' has invalid name '-bad-name': a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')"), + errors.New("webhook of type 'ValidatingAdmissionWebhook' has invalid name 'a-name-is-waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay-too-long': must be no more than 253 characters"), + errors.New("webhook of type 'ValidatingAdmissionWebhook' has invalid name 'b-name-is-waaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaay-too-long-and-bad-': a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'),must be no more than 253 characters"), + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := validators.CheckWebhookNameIsDNS1123SubDomain(tc.bundle) + require.Equal(t, tc.expectedErrs, errs) + }) + } +} From 6bc2421aef6640fa8ad169872a149478efe93380 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Mon, 12 May 2025 13:10:20 +0100 Subject: [PATCH 08/11] Add v0 certificate parity Signed-off-by: Per G. da Silva --- .../render/certproviders/certmanager.go | 79 ++++++++++++++++++- .../render/certproviders/certmanager_test.go | 13 ++- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/internal/operator-controller/rukpak/render/certproviders/certmanager.go b/internal/operator-controller/rukpak/render/certproviders/certmanager.go index f8b7d06a1..f9dada3f0 100644 --- a/internal/operator-controller/rukpak/render/certproviders/certmanager.go +++ b/internal/operator-controller/rukpak/render/certproviders/certmanager.go @@ -3,6 +3,7 @@ package certproviders import ( "errors" "fmt" + "time" certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" @@ -18,6 +19,7 @@ import ( const ( certManagerInjectCAAnnotation = "cert-manager.io/inject-ca-from" + olmv0RotationPeriod = 730 * 24 * time.Hour // 2 year rotation ) var _ render.CertificateProvider = (*CertManagerCertificateProvider)(nil) @@ -50,6 +52,72 @@ func (p CertManagerCertificateProvider) AdditionalObjects(cfg render.Certificate errs []error ) + // OLMv0 parity: + // - self-signed issuer + // - 2 year rotation period + // - CN: argocd-operator-controller-manager-service.argocd (-service.) + // - CA: false + // - DNS:argocd-operator-controller-manager-service.argocd, DNS:argocd-operator-controller-manager-service.argocd.svc, DNS:argocd-operator-controller-manager-service.argocd.svc.cluster.local + + // Full example of OLMv0 Certificate data (argocd-operator.v0.8.0): + //Certificate: + // Data: + // Version: 3 (0x2) + // Serial Number: 1507821748758744637 (0x14ecdbe4475f8e3d) + // Signature Algorithm: ecdsa-with-SHA256 + // Issuer: O=Red Hat, Inc., CN=olm-selfsigned-275dd2a363db7513 + // Validity + // Not Before: May 12 11:15:02 2025 GMT + // Not After : May 12 11:15:02 2027 GMT + // Subject: O=Red Hat, Inc., CN=argocd-operator-controller-manager-service.argocd + // Subject Public Key Info: + // Public Key Algorithm: id-ecPublicKey + // Public-Key: (256 bit) + // pub: ... + // ASN1 OID: prime256v1 + // NIST CURVE: P-256 + // X509v3 extensions: + // X509v3 Extended Key Usage: + // TLS Web Server Authentication + // X509v3 Basic Constraints: critical + // CA:FALSE + // X509v3 Authority Key Identifier: ... + // X509v3 Subject Alternative Name: + // DNS:argocd-operator-controller-manager-service.argocd, DNS:argocd-operator-controller-manager-service.argocd.svc, DNS:argocd-operator-controller-manager-service.argocd.svc.cluster.local + // Signature Algorithm: ecdsa-with-SHA256 + // Signature Value: ... + + // Full example of OLMv1 certificate for argocd-operator v0.8.0 with the Issuer and Certificate settings that follow: + //Certificate: + // Data: + // Version: 3 (0x2) + // Serial Number: + // d5:8f:4f:ae:b1:67:59:9d:fe:53:b5:41:d3:10:5a:2b + // Signature Algorithm: sha256WithRSAEncryption + // Issuer: CN=argocd-operator-controller-manager-service.argocd + // Validity + // Not Before: May 12 11:55:28 2025 GMT + // Not After : May 12 11:55:28 2027 GMT + // Subject: CN=argocd-operator-controller-manager-service.argocd + // Subject Public Key Info: + // Public Key Algorithm: rsaEncryption + // Public-Key: (2048 bit) + // Modulus: ... + // Exponent: 65537 (0x10001) + // X509v3 extensions: + // X509v3 Extended Key Usage: + // TLS Web Server Authentication + // X509v3 Basic Constraints: critical + // CA:FALSE + // X509v3 Subject Alternative Name: + // DNS:argocd-operator-controller-manager-service.argocd, DNS:argocd-operator-controller-manager-service.argocd.svc, DNS:argocd-operator-controller-manager-service.argocd.svc.cluster.local + // Signature Algorithm: sha256WithRSAEncryption + // Signature Value: ... + + // Notes: + // - the Organization "Red Hat, Inc." will not be used to avoid any hard links between Red Hat and the operator-controller project + // - for OLMv1 we'll use the default algorithm settings and key size (2048) coming from cert-manager as this is deemed more secure + issuer := &certmanagerv1.Issuer{ TypeMeta: metav1.TypeMeta{ APIVersion: certmanagerv1.SchemeGroupVersion.String(), @@ -83,11 +151,20 @@ func (p CertManagerCertificateProvider) AdditionalObjects(cfg render.Certificate }, Spec: certmanagerv1.CertificateSpec{ SecretName: cfg.CertName, + CommonName: fmt.Sprintf("%s.%s", cfg.WebhookServiceName, cfg.Namespace), Usages: []certmanagerv1.KeyUsage{certmanagerv1.UsageServerAuth}, - DNSNames: []string{fmt.Sprintf("%s.%s.svc", cfg.WebhookServiceName, cfg.Namespace)}, + IsCA: false, + DNSNames: []string{ + fmt.Sprintf("%s.%s", cfg.WebhookServiceName, cfg.Namespace), + fmt.Sprintf("%s.%s.svc", cfg.WebhookServiceName, cfg.Namespace), + fmt.Sprintf("%s.%s.svc.cluster.local", cfg.WebhookServiceName, cfg.Namespace), + }, IssuerRef: certmanagermetav1.ObjectReference{ Name: issuer.GetName(), }, + Duration: &metav1.Duration{ + Duration: olmv0RotationPeriod, + }, }, } certObj, err := util.ToUnstructured(certificate) diff --git a/internal/operator-controller/rukpak/render/certproviders/certmanager_test.go b/internal/operator-controller/rukpak/render/certproviders/certmanager_test.go index 08570d12f..b5da581d3 100644 --- a/internal/operator-controller/rukpak/render/certproviders/certmanager_test.go +++ b/internal/operator-controller/rukpak/render/certproviders/certmanager_test.go @@ -2,6 +2,7 @@ package certproviders_test import ( "testing" + "time" certmanagerv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" certmanagermetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" @@ -128,10 +129,20 @@ func Test_CertManagerProvider_AdditionalObjects(t *testing.T) { Spec: certmanagerv1.CertificateSpec{ SecretName: "cert-name", Usages: []certmanagerv1.KeyUsage{certmanagerv1.UsageServerAuth}, - DNSNames: []string{"webhook-service.namespace.svc"}, + CommonName: "webhook-service.namespace", + IsCA: false, + DNSNames: []string{ + "webhook-service.namespace", + "webhook-service.namespace.svc", + "webhook-service.namespace.svc.cluster.local", + }, IssuerRef: certmanagermetav1.ObjectReference{ Name: "cert-name-selfsigned-issuer", }, + Duration: &metav1.Duration{ + // OLMv0 has a 2 year certificate rotation period + Duration: 730 * 24 * time.Hour, + }, }, }), }, objs) From 28d5ffcaac1275b2f3ddb722f86bc16a21649358 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Tue, 13 May 2025 08:40:34 +0100 Subject: [PATCH 09/11] Update featureflag to WebhookProviderCertManager Signed-off-by: Per G. da Silva --- internal/operator-controller/features/features.go | 9 +++++---- .../operator-controller/rukpak/convert/registryv1.go | 2 +- .../rukpak/convert/registryv1_test.go | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/operator-controller/features/features.go b/internal/operator-controller/features/features.go index ea4f68a85..9101bb7f6 100644 --- a/internal/operator-controller/features/features.go +++ b/internal/operator-controller/features/features.go @@ -14,7 +14,7 @@ const ( PreflightPermissions featuregate.Feature = "PreflightPermissions" SingleOwnNamespaceInstallSupport featuregate.Feature = "SingleOwnNamespaceInstallSupport" SyntheticPermissions featuregate.Feature = "SyntheticPermissions" - WebhookSupport featuregate.Feature = "WebhookSupport" + WebhookProviderCertManager featuregate.Feature = "WebhookProviderCertManager" ) var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -43,10 +43,11 @@ var operatorControllerFeatureGates = map[featuregate.Feature]featuregate.Feature LockToDefault: false, }, - // WebhookSupport enables support for installing + // WebhookProviderCertManager enables support for installing // registry+v1 cluster extensions that include validating, - // mutating, and/or conversion webhooks - WebhookSupport: { + // mutating, and/or conversion webhooks with CertManager + // as the certificate provider. + WebhookProviderCertManager: { Default: false, PreRelease: featuregate.Alpha, LockToDefault: false, diff --git a/internal/operator-controller/rukpak/convert/registryv1.go b/internal/operator-controller/rukpak/convert/registryv1.go index aa17a08e2..d2eb4bc3a 100644 --- a/internal/operator-controller/rukpak/convert/registryv1.go +++ b/internal/operator-controller/rukpak/convert/registryv1.go @@ -263,7 +263,7 @@ func (c Converter) Convert(rv1 render.RegistryV1, installNamespace string, targe return nil, fmt.Errorf("apiServiceDefintions are not supported") } - if !features.OperatorControllerFeatureGate.Enabled(features.WebhookSupport) && len(rv1.CSV.Spec.WebhookDefinitions) > 0 { + if !features.OperatorControllerFeatureGate.Enabled(features.WebhookProviderCertManager) && len(rv1.CSV.Spec.WebhookDefinitions) > 0 { return nil, fmt.Errorf("webhookDefinitions are not supported") } diff --git a/internal/operator-controller/rukpak/convert/registryv1_test.go b/internal/operator-controller/rukpak/convert/registryv1_test.go index 29a7f6217..e1c69fd7a 100644 --- a/internal/operator-controller/rukpak/convert/registryv1_test.go +++ b/internal/operator-controller/rukpak/convert/registryv1_test.go @@ -563,7 +563,7 @@ func TestRegistryV1SuiteGenerateNoWebhooks(t *testing.T) { } func TestRegistryV1SuiteGenerateWebhooks_WebhookSupportFGEnabled(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.WebhookSupport, true) + featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.WebhookProviderCertManager, true) t.Log("RegistryV1 Suite Convert") t.Log("It should generate objects successfully based on target namespaces") From b3b96582cdcf721cd9a03a50d78cfafe9282b04b Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Tue, 13 May 2025 11:00:18 +0100 Subject: [PATCH 10/11] Move install mode validation to renderer Signed-off-by: Per G. da Silva --- .../rukpak/convert/registryv1.go | 27 ---- .../rukpak/convert/registryv1_test.go | 134 +---------------- .../rukpak/render/render.go | 58 +++++++- .../rukpak/render/render_test.go | 135 +++++++++++++++++- 4 files changed, 188 insertions(+), 166 deletions(-) diff --git a/internal/operator-controller/rukpak/convert/registryv1.go b/internal/operator-controller/rukpak/convert/registryv1.go index d2eb4bc3a..7c87b7783 100644 --- a/internal/operator-controller/rukpak/convert/registryv1.go +++ b/internal/operator-controller/rukpak/convert/registryv1.go @@ -191,29 +191,6 @@ func copyMetadataPropertiesToCSV(csv *v1alpha1.ClusterServiceVersion, fsys fs.FS return nil } -func validateTargetNamespaces(supportedInstallModes sets.Set[string], installNamespace string, targetNamespaces []string) error { - set := sets.New[string](targetNamespaces...) - switch { - case set.Len() == 0 || (set.Len() == 1 && set.Has("")): - if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) { - return nil - } - return fmt.Errorf("supported install modes %v do not support targeting all namespaces", sets.List(supportedInstallModes)) - case set.Len() == 1 && !set.Has(""): - if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeSingleNamespace)) { - return nil - } - if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNamespaces[0] == installNamespace { - return nil - } - default: - if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeMultiNamespace)) && !set.Has("") { - return nil - } - } - return fmt.Errorf("supported install modes %v do not support target namespaces %v", sets.List[string](supportedInstallModes), targetNamespaces) -} - var PlainConverter = Converter{ BundleRenderer: render.BundleRenderer{ BundleValidator: validators.RegistryV1BundleValidator, @@ -255,10 +232,6 @@ func (c Converter) Convert(rv1 render.RegistryV1, installNamespace string, targe } } - if err := validateTargetNamespaces(supportedInstallModes, installNamespace, targetNamespaces); err != nil { - return nil, err - } - if len(rv1.CSV.Spec.APIServiceDefinitions.Owned) > 0 { return nil, fmt.Errorf("apiServiceDefintions are not supported") } diff --git a/internal/operator-controller/rukpak/convert/registryv1_test.go b/internal/operator-controller/rukpak/convert/registryv1_test.go index e1c69fd7a..c87f3a4c5 100644 --- a/internal/operator-controller/rukpak/convert/registryv1_test.go +++ b/internal/operator-controller/rukpak/convert/registryv1_test.go @@ -244,138 +244,6 @@ func getBaseCsvAndService() (v1alpha1.ClusterServiceVersion, corev1.Service) { return baseCSV, svc } -func TestRegistryV1SuiteGenerateAllNamespace(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should convert into plain manifests successfully with AllNamespaces") - baseCSV, svc := getBaseCsvAndService() - csv := baseCSV.DeepCopy() - csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}} - - t.Log("By creating a registry v1 bundle") - watchNamespaces := []string{""} - unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: *csv, - Others: []unstructured.Unstructured{unstructuredSvc}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces) - require.NoError(t, err) - - t.Log("By verifying if plain bundle has required objects") - require.NotNil(t, plainBundle) - require.Len(t, plainBundle.Objects, 5) - - t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template") - dep := findObjectByName("test-deployment", plainBundle.Objects) - require.NotNil(t, dep) - require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces) - require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces]) -} - -func TestRegistryV1SuiteGenerateMultiNamespace(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should convert into plain manifests successfully with MultiNamespace") - baseCSV, svc := getBaseCsvAndService() - csv := baseCSV.DeepCopy() - csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}} - - t.Log("By creating a registry v1 bundle") - watchNamespaces := []string{"testWatchNs1", "testWatchNs2"} - unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: *csv, - Others: []unstructured.Unstructured{unstructuredSvc}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces) - require.NoError(t, err) - - t.Log("By verifying if plain bundle has required objects") - require.NotNil(t, plainBundle) - require.Len(t, plainBundle.Objects, 7) - - t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template") - dep := findObjectByName("test-deployment", plainBundle.Objects) - require.NotNil(t, dep) - require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces) - require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces]) -} - -func TestRegistryV1SuiteGenerateSingleNamespace(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should convert into plain manifests successfully with SingleNamespace") - baseCSV, svc := getBaseCsvAndService() - csv := baseCSV.DeepCopy() - csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}} - - t.Log("By creating a registry v1 bundle") - watchNamespaces := []string{"testWatchNs1"} - unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: *csv, - Others: []unstructured.Unstructured{unstructuredSvc}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces) - require.NoError(t, err) - - t.Log("By verifying if plain bundle has required objects") - require.NotNil(t, plainBundle) - require.Len(t, plainBundle.Objects, 5) - - t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template") - dep := findObjectByName("test-deployment", plainBundle.Objects) - require.NotNil(t, dep) - require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces) - require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces]) -} - -func TestRegistryV1SuiteGenerateOwnNamespace(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should convert into plain manifests successfully with own namespace") - baseCSV, svc := getBaseCsvAndService() - csv := baseCSV.DeepCopy() - csv.Spec.InstallModes = []v1alpha1.InstallMode{{Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}} - - t.Log("By creating a registry v1 bundle") - watchNamespaces := []string{installNamespace} - unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: *csv, - Others: []unstructured.Unstructured{unstructuredSvc}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, installNamespace, watchNamespaces) - require.NoError(t, err) - - t.Log("By verifying if plain bundle has required objects") - require.NotNil(t, plainBundle) - require.Len(t, plainBundle.Objects, 5) - - t.Log("By verifying olm.targetNamespaces annotation in the deployment's pod template") - dep := findObjectByName("test-deployment", plainBundle.Objects) - require.NotNil(t, dep) - require.Contains(t, dep.(*appsv1.Deployment).Spec.Template.Annotations, olmNamespaces) - require.Equal(t, strings.Join(watchNamespaces, ","), dep.(*appsv1.Deployment).Spec.Template.Annotations[olmNamespaces]) -} - func TestConvertInstallModeValidation(t *testing.T) { for _, tc := range []struct { description string @@ -609,7 +477,7 @@ func TestRegistryV1SuiteGenerateWebhooks_WebhookSupportFGEnabled(t *testing.T) { require.NotNil(t, plainBundle) } -func TestRegistryV1SuiteGenerateNoAPISerciceDefinitions(t *testing.T) { +func TestRegistryV1SuiteGenerateNoAPIServiceDefinitions(t *testing.T) { t.Log("RegistryV1 Suite Convert") t.Log("It should generate objects successfully based on target namespaces") diff --git a/internal/operator-controller/rukpak/render/render.go b/internal/operator-controller/rukpak/render/render.go index 7e7a599bf..a9dbb6a84 100644 --- a/internal/operator-controller/rukpak/render/render.go +++ b/internal/operator-controller/rukpak/render/render.go @@ -2,10 +2,12 @@ package render import ( "errors" + "fmt" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/api/pkg/operators/v1alpha1" @@ -78,6 +80,20 @@ func (o *Options) apply(opts ...Option) *Options { return o } +func (o *Options) validate(rv1 *RegistryV1) (*Options, []error) { + var errs []error + if len(o.TargetNamespaces) == 0 { + errs = append(errs, errors.New("at least one target namespace must be specified")) + } + if o.UniqueNameGenerator == nil { + errs = append(errs, errors.New("unique name generator must be specified")) + } + if err := validateTargetNamespaces(rv1, o.InstallNamespace, o.TargetNamespaces); err != nil { + errs = append(errs, fmt.Errorf("invalid target namespaces %v: %w", o.TargetNamespaces, err)) + } + return o, errs +} + type Option func(*Options) func WithTargetNamespaces(namespaces ...string) Option { @@ -109,13 +125,19 @@ func (r BundleRenderer) Render(rv1 RegistryV1, installNamespace string, opts ... return nil, err } - genOpts := (&Options{ + // generate bundle objects + genOpts, errs := (&Options{ + // default options InstallNamespace: installNamespace, TargetNamespaces: []string{metav1.NamespaceAll}, UniqueNameGenerator: DefaultUniqueNameGenerator, - }).apply(opts...) + CertificateProvider: nil, + }).apply(opts...).validate(&rv1) + + if len(errs) > 0 { + return nil, fmt.Errorf("invalid option(s): %w", errors.Join(errs...)) + } - // generate bundle objects objs, err := ResourceGenerators(r.ResourceGenerators).GenerateResources(&rv1, *genOpts) if err != nil { return nil, err @@ -131,3 +153,33 @@ func DefaultUniqueNameGenerator(base string, o interface{}) (string, error) { } return util.ObjectNameForBaseAndSuffix(base, hashStr), nil } + +func validateTargetNamespaces(rv1 *RegistryV1, installNamespace string, targetNamespaces []string) error { + supportedInstallModes := sets.New[string]() + for _, im := range rv1.CSV.Spec.InstallModes { + if im.Supported { + supportedInstallModes.Insert(string(im.Type)) + } + } + + set := sets.New[string](targetNamespaces...) + switch { + case set.Len() == 0 || (set.Len() == 1 && set.Has("")): + if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeAllNamespaces)) { + return nil + } + return fmt.Errorf("supported install modes %v do not support targeting all namespaces", sets.List(supportedInstallModes)) + case set.Len() == 1 && !set.Has(""): + if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeSingleNamespace)) { + return nil + } + if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeOwnNamespace)) && targetNamespaces[0] == installNamespace { + return nil + } + default: + if supportedInstallModes.Has(string(v1alpha1.InstallModeTypeMultiNamespace)) && !set.Has("") { + return nil + } + } + return fmt.Errorf("supported install modes %v do not support target namespaces %v", sets.List[string](supportedInstallModes), targetNamespaces) +} diff --git a/internal/operator-controller/rukpak/render/render_test.go b/internal/operator-controller/rukpak/render/render_test.go index 07409ed5b..23455a8be 100644 --- a/internal/operator-controller/rukpak/render/render_test.go +++ b/internal/operator-controller/rukpak/render/render_test.go @@ -11,13 +11,18 @@ import ( corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/operator-framework/api/pkg/operators/v1alpha1" + "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" ) func Test_BundleRenderer_NoConfig(t *testing.T) { renderer := render.BundleRenderer{} - objs, err := renderer.Render(render.RegistryV1{}, "", nil) + objs, err := renderer.Render( + render.RegistryV1{ + CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + }, "", nil) require.NoError(t, err) require.Empty(t, objs) } @@ -55,6 +60,124 @@ func Test_BundleRenderer_CreatesCorrectDefaultOptions(t *testing.T) { _, _ = renderer.Render(render.RegistryV1{}, expectedInstallNamespace) } +func Test_BundleRenderer_ValidatesRenderOptions(t *testing.T) { + for _, tc := range []struct { + name string + installNamespace string + csv v1alpha1.ClusterServiceVersion + opts []render.Option + err error + }{ + { + name: "rejects empty targetNamespaces", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + opts: []render.Option{ + render.WithTargetNamespaces(), + }, + err: errors.New("invalid option(s): at least one target namespace must be specified"), + }, { + name: "rejects nil unique name generator", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + opts: []render.Option{ + render.WithUniqueNameGenerator(nil), + }, + err: errors.New("invalid option(s): unique name generator must be specified"), + }, { + name: "rejects all namespace install if AllNamespaces install mode is not supported", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace)), + opts: []render.Option{ + render.WithTargetNamespaces(corev1.NamespaceAll), + }, + err: errors.New("invalid option(s): invalid target namespaces []: supported install modes [SingleNamespace] do not support targeting all namespaces"), + }, { + name: "rejects own namespace install if only AllNamespace install mode is supported", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + opts: []render.Option{ + render.WithTargetNamespaces("install-namespace"), + }, + err: errors.New("invalid option(s): invalid target namespaces [install-namespace]: supported install modes [AllNamespaces] do not support target namespaces [install-namespace]"), + }, { + name: "rejects install out of own namespace if only OwnNamespace install mode is supported", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace)), + opts: []render.Option{ + render.WithTargetNamespaces("not-install-namespace"), + }, + err: errors.New("invalid option(s): invalid target namespaces [not-install-namespace]: supported install modes [OwnNamespace] do not support target namespaces [not-install-namespace]"), + }, { + name: "rejects multi-namespace install if MultiNamespace install mode is not supported", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + opts: []render.Option{ + render.WithTargetNamespaces("ns1", "ns2", "ns3"), + }, + err: errors.New("invalid option(s): invalid target namespaces [ns1 ns2 ns3]: supported install modes [AllNamespaces] do not support target namespaces [ns1 ns2 ns3]"), + }, { + name: "rejects if bundle supports no install modes", + installNamespace: "install-namespace", + csv: MakeCSV(), + opts: []render.Option{ + render.WithTargetNamespaces("some-namespace"), + }, + err: errors.New("invalid option(s): invalid target namespaces [some-namespace]: supported install modes [] do not support target namespaces [some-namespace]"), + }, { + name: "accepts all namespace render if AllNamespaces install mode is supported", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + opts: []render.Option{ + render.WithTargetNamespaces(""), + }, + }, { + name: "accepts install namespace render if SingleNamespace install mode is supported", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace)), + opts: []render.Option{ + render.WithTargetNamespaces("some-namespace"), + }, + }, { + name: "accepts all install namespace render if OwnNamespace install mode is supported", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeOwnNamespace)), + opts: []render.Option{ + render.WithTargetNamespaces("install-namespace"), + }, + }, { + name: "accepts single namespace render if SingleNamespace install mode is supported", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace)), + opts: []render.Option{ + render.WithTargetNamespaces("some-namespace"), + }, + }, { + name: "accepts multi namespace render if MultiNamespace install mode is supported", + installNamespace: "install-namespace", + csv: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeMultiNamespace)), + opts: []render.Option{ + render.WithTargetNamespaces("n1", "n2", "n3"), + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + renderer := render.BundleRenderer{} + _, err := renderer.Render( + render.RegistryV1{CSV: tc.csv}, + tc.installNamespace, + tc.opts..., + ) + if tc.err == nil { + require.NoError(t, err) + } else { + require.Error(t, err) + require.Equal(t, tc.err.Error(), err.Error()) + } + }) + } +} + func Test_BundleRenderer_AppliesUserOptions(t *testing.T) { isOptionApplied := false _, _ = render.BundleRenderer{}.Render(render.RegistryV1{}, "install-namespace", func(options *render.Options) { @@ -100,7 +223,10 @@ func Test_BundleRenderer_CallsResourceGenerators(t *testing.T) { }, }, } - objs, err := renderer.Render(render.RegistryV1{}, "") + objs, err := renderer.Render( + render.RegistryV1{ + CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + }, "") require.NoError(t, err) require.Equal(t, []client.Object{&corev1.Namespace{}, &corev1.Service{}, &appsv1.Deployment{}}, objs) } @@ -116,7 +242,10 @@ func Test_BundleRenderer_ReturnsResourceGeneratorErrors(t *testing.T) { }, }, } - objs, err := renderer.Render(render.RegistryV1{}, "") + objs, err := renderer.Render( + render.RegistryV1{ + CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), + }, "") require.Nil(t, objs) require.Error(t, err) require.Contains(t, err.Error(), "generator error") From 588a9d600810f56d5f1aa013b0eac42da51c5549 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Tue, 13 May 2025 11:01:44 +0100 Subject: [PATCH 11/11] Remove superfluous resource generation tests from convert package Signed-off-by: Per G. da Silva --- .../rukpak/convert/registryv1_test.go | 1281 +---------------- 1 file changed, 14 insertions(+), 1267 deletions(-) diff --git a/internal/operator-controller/rukpak/convert/registryv1_test.go b/internal/operator-controller/rukpak/convert/registryv1_test.go index c87f3a4c5..516bc1da2 100644 --- a/internal/operator-controller/rukpak/convert/registryv1_test.go +++ b/internal/operator-controller/rukpak/convert/registryv1_test.go @@ -1,7 +1,6 @@ package convert_test import ( - "fmt" "io/fs" "os" "strings" @@ -9,28 +8,22 @@ import ( "testing/fstest" "github.com/stretchr/testify/require" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - rbacv1 "k8s.io/api/rbac/v1" schedulingv1 "k8s.io/api/scheduling/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/api/pkg/operators/v1alpha1" - "github.com/operator-framework/operator-registry/alpha/property" "github.com/operator-framework/operator-controller/internal/operator-controller/features" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/validators" . "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/util/testing" - filterutil "github.com/operator-framework/operator-controller/internal/shared/util/filter" ) const ( @@ -67,7 +60,7 @@ func TestRegistryV1SuiteNamespaceNotAvailable(t *testing.T) { t.Log("By creating a registry v1 bundle") csv, svc := getCsvAndService() - unstructuredSvc := convertToUnstructured(t, svc) + unstructuredSvc := *ToUnstructuredT(t, &svc) registryv1Bundle := render.RegistryV1{ PackageName: "testPkg", CSV: csv, @@ -99,7 +92,7 @@ func TestRegistryV1SuiteNamespaceAvailable(t *testing.T) { csv, svc := getCsvAndService() svc.SetNamespace("otherNs") - unstructuredSvc := convertToUnstructured(t, svc) + unstructuredSvc := *ToUnstructuredT(t, &svc) unstructuredSvc.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"}) registryv1Bundle := render.RegistryV1{ @@ -134,13 +127,17 @@ func TestRegistryV1SuiteNamespaceUnsupportedKind(t *testing.T) { csv, _ := getCsvAndService() t.Log("By creating an unsupported kind") - event := corev1.Event{ + event := &corev1.Event{ + TypeMeta: metav1.TypeMeta{ + APIVersion: corev1.SchemeGroupVersion.String(), + Kind: "Event", + }, ObjectMeta: metav1.ObjectMeta{ Name: "testEvent", }, } - unstructuredEvt := convertToUnstructured(t, event) + unstructuredEvt := *ToUnstructuredT(t, event) unstructuredEvt.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Event"}) registryv1Bundle := render.RegistryV1{ @@ -168,13 +165,17 @@ func TestRegistryV1SuiteNamespaceClusterScoped(t *testing.T) { csv, _ := getCsvAndService() t.Log("By creating an unsupported kind") - pc := schedulingv1.PriorityClass{ + pc := &schedulingv1.PriorityClass{ + TypeMeta: metav1.TypeMeta{ + APIVersion: schedulingv1.SchemeGroupVersion.String(), + Kind: "PriorityClass", + }, ObjectMeta: metav1.ObjectMeta{ Name: "testPriorityClass", }, } - unstructuredpriorityclass := convertToUnstructured(t, pc) + unstructuredpriorityclass := *ToUnstructuredT(t, pc) unstructuredpriorityclass.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "PriorityClass"}) registryv1Bundle := render.RegistryV1{ @@ -197,151 +198,6 @@ func TestRegistryV1SuiteNamespaceClusterScoped(t *testing.T) { require.Empty(t, resObj.GetNamespace()) } -func getBaseCsvAndService() (v1alpha1.ClusterServiceVersion, corev1.Service) { - // base CSV definition that each test case will deep copy and modify - baseCSV := MakeCSV( - WithName("testCSV"), - WithAnnotations(map[string]string{ - olmProperties: fmt.Sprintf("[{\"type\": %s, \"value\": \"%s\"}]", property.TypeConstraint, "value"), - }), - WithStrategyDeploymentSpecs( - v1alpha1.StrategyDeploymentSpec{ - Name: "test-deployment", - Spec: appsv1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "testContainer", - Image: "testImage", - }, - }, - }, - }, - }, - }, - ), - WithPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "testServiceAccount", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"test"}, - Resources: []string{"pods"}, - Verbs: []string{"*"}, - }, - }, - }, - ), - ) - - svc := corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testService", - }, - } - svc.SetGroupVersionKind(schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Service"}) - return baseCSV, svc -} - -func TestConvertInstallModeValidation(t *testing.T) { - for _, tc := range []struct { - description string - installModes []v1alpha1.InstallMode - installNamespace string - watchNamespaces []string - }{ - { - description: "fails on AllNamespaces install mode when CSV does not support it", - installNamespace: "install-namespace", - watchNamespaces: []string{corev1.NamespaceAll}, - installModes: []v1alpha1.InstallMode{ - {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: false}, - {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, - {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}, - {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, - }, - }, { - description: "fails on SingleNamespace install mode when CSV does not support it", - installNamespace: "install-namespace", - watchNamespaces: []string{"watch-namespace"}, - installModes: []v1alpha1.InstallMode{ - {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}, - {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, - {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}, - {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, - }, - }, { - description: "fails on OwnNamespace install mode when CSV does not support it and watch namespace is not install namespace", - installNamespace: "install-namespace", - watchNamespaces: []string{"watch-namespace"}, - installModes: []v1alpha1.InstallMode{ - {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}, - {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, - {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}, - {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, - }, - }, { - description: "fails on MultiNamespace install mode when CSV does not support it", - installNamespace: "install-namespace", - watchNamespaces: []string{"watch-namespace-one", "watch-namespace-two", "watch-namespace-three"}, - installModes: []v1alpha1.InstallMode{ - {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: true}, - {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: true}, - {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: true}, - {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: false}, - }, - }, { - description: "fails on MultiNamespace install mode when CSV supports it but watchNamespaces is empty", - installNamespace: "install-namespace", - watchNamespaces: []string{}, - installModes: []v1alpha1.InstallMode{ - // because install mode is inferred by the watchNamespaces parameter - // force MultiNamespace install by disabling other modes - {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: false}, - {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: false}, - {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}, - {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, - }, - }, { - description: "fails on MultiNamespace install mode when CSV supports it but watchNamespaces is nil", - installNamespace: "install-namespace", - watchNamespaces: nil, - installModes: []v1alpha1.InstallMode{ - // because install mode is inferred by the watchNamespaces parameter - // force MultiNamespace install by disabling other modes - {Type: v1alpha1.InstallModeTypeAllNamespaces, Supported: false}, - {Type: v1alpha1.InstallModeTypeOwnNamespace, Supported: false}, - {Type: v1alpha1.InstallModeTypeSingleNamespace, Supported: false}, - {Type: v1alpha1.InstallModeTypeMultiNamespace, Supported: true}, - }, - }, - } { - t.Run(tc.description, func(t *testing.T) { - t.Log("RegistryV1 Suite Convert") - t.Log("It should generate objects successfully based on target namespaces") - - t.Log("It should error when all namespace mode is disabled with target namespace containing an empty string") - baseCSV, svc := getBaseCsvAndService() - csv := baseCSV.DeepCopy() - csv.Spec.InstallModes = tc.installModes - - t.Log("By creating a registry v1 bundle") - unstructuredSvc := convertToUnstructured(t, svc) - registryv1Bundle := render.RegistryV1{ - PackageName: "testPkg", - CSV: *csv, - Others: []unstructured.Unstructured{unstructuredSvc}, - } - - t.Log("By converting to plain") - plainBundle, err := convert.PlainConverter.Convert(registryv1Bundle, tc.installNamespace, tc.watchNamespaces) - require.Error(t, err) - require.Nil(t, plainBundle) - }) - } -} - func TestRegistryV1SuiteReadBundleFileSystem(t *testing.T) { t.Log("RegistryV1 Suite Convert") t.Log("It should generate objects successfully based on target namespaces") @@ -508,1107 +364,6 @@ func TestRegistryV1SuiteGenerateNoAPIServiceDefinitions(t *testing.T) { require.Nil(t, plainBundle) } -func Test_Convert_DeploymentResourceGeneration(t *testing.T) { - for _, tc := range []struct { - name string - bundle render.RegistryV1 - installNamespace string - targetNamespaces []string - expectedResources []client.Object - }{ - { - name: "generates deployment resources", - installNamespace: "install-namespace", - targetNamespaces: []string{""}, - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), - WithAnnotations(map[string]string{ - "csv": "annotation", - }), - WithStrategyDeploymentSpecs( - v1alpha1.StrategyDeploymentSpec{ - Name: "deployment-one", - Label: map[string]string{ - "bar": "foo", - }, - Spec: appsv1.DeploymentSpec{ - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "pod": "annotation", - }, - }, - Spec: corev1.PodSpec{ - ServiceAccountName: "some-service-account", - }, - }, - }, - }, - v1alpha1.StrategyDeploymentSpec{ - Name: "deployment-two", - Spec: appsv1.DeploymentSpec{}, - }, - ), - ), - }, - expectedResources: []client.Object{ - &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - Kind: "Deployment", - APIVersion: appsv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "install-namespace", - Name: "deployment-one", - Labels: map[string]string{ - "bar": "foo", - }, - }, - Spec: appsv1.DeploymentSpec{ - RevisionHistoryLimit: ptr.To(int32(1)), - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "csv": "annotation", - "olm.targetNamespaces": "", - "pod": "annotation", - }, - }, - Spec: corev1.PodSpec{ - ServiceAccountName: "some-service-account", - }, - }, - }, - }, - &appsv1.Deployment{ - TypeMeta: metav1.TypeMeta{ - Kind: "Deployment", - APIVersion: appsv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "install-namespace", - Name: "deployment-two", - }, - Spec: appsv1.DeploymentSpec{ - RevisionHistoryLimit: ptr.To(int32(1)), - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "csv": "annotation", - "olm.targetNamespaces": "", - }, - }, - }, - }, - }, - }, - }, - } { - t.Run(tc.name, func(t *testing.T) { - // ignore bundle validation for these unit tests as we only want to test - // the specific resource generation logic - conv := convert.PlainConverter - conv.BundleValidator = nil - plain, err := conv.Convert(tc.bundle, tc.installNamespace, tc.targetNamespaces) - require.NoError(t, err) - for _, expectedObj := range tc.expectedResources { - // find object in generated objects - result := filterutil.Filter(plain.Objects, byTargetObject(expectedObj)) - require.Len(t, result, 1) - require.Equal(t, expectedObj, result[0]) - } - }) - } -} - -func Test_Convert_RoleRoleBindingResourceGeneration(t *testing.T) { - for _, tc := range []struct { - name string - installNamespace string - targetNamespaces []string - bundle render.RegistryV1 - expectedResources []client.Object - }{ - { - name: "does not generate any resources when in AllNamespaces mode (target namespace is [''])", - installNamespace: "install-namespace", - targetNamespaces: []string{metav1.NamespaceAll}, - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), - WithName("csv"), - WithPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-one", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - ), - ), - }, - expectedResources: nil, - }, - { - name: "generates role and rolebinding for permission service-account when in Single/OwnNamespace mode (target namespace contains a single namespace)", - installNamespace: "install-namespace", - targetNamespaces: []string{"watch-namespace"}, - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithName("csv"), - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), - WithPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-one", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - ), - ), - }, - expectedResources: []client.Object{ - &rbacv1.Role{ - TypeMeta: metav1.TypeMeta{ - Kind: "Role", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace", - Name: "csv-service-accoun-1qo4d10qruxvfkjf58b5cymauit0vk33tu31q7au0p6k", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - &rbacv1.RoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "RoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace", - Name: "csv-service-accoun-1qo4d10qruxvfkjf58b5cymauit0vk33tu31q7au0p6k", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "service-account-one", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "Role", - Name: "csv-service-accoun-1qo4d10qruxvfkjf58b5cymauit0vk33tu31q7au0p6k", - }, - }, - }, - }, - { - name: "generates role and rolebinding for permission service-account for each target namespace when in MultiNamespace install mode (target namespace contains multiple namespaces)", - installNamespace: "install-namespace", - targetNamespaces: []string{"watch-namespace", "watch-namespace-two"}, - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeMultiNamespace), - WithName("csv"), - WithPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-one", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - ), - ), - }, - expectedResources: []client.Object{ - &rbacv1.Role{ - TypeMeta: metav1.TypeMeta{ - Kind: "Role", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace", - Name: "csv-service-accoun-1qo4d10qruxvfkjf58b5cymauit0vk33tu31q7au0p6k", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - &rbacv1.RoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "RoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace", - Name: "csv-service-accoun-1qo4d10qruxvfkjf58b5cymauit0vk33tu31q7au0p6k", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "service-account-one", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "Role", - Name: "csv-service-accoun-1qo4d10qruxvfkjf58b5cymauit0vk33tu31q7au0p6k", - }, - }, - &rbacv1.Role{ - TypeMeta: metav1.TypeMeta{ - Kind: "Role", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace-two", - Name: "csv-service-accoun-1qo4d10qruxvfkjf58b5cymauit0vk33tu31q7au0p6k", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - &rbacv1.RoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "RoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace-two", - Name: "csv-service-accoun-1qo4d10qruxvfkjf58b5cymauit0vk33tu31q7au0p6k", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "service-account-one", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "Role", - Name: "csv-service-accoun-1qo4d10qruxvfkjf58b5cymauit0vk33tu31q7au0p6k", - }, - }, - }, - }, - { - name: "generates role and rolebinding for each permission service-account", - installNamespace: "install-namespace", - targetNamespaces: []string{"watch-namespace"}, - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeAllNamespaces), - WithName("csv"), - WithPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-one", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-two", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - ), - ), - }, - expectedResources: []client.Object{ - &rbacv1.Role{ - TypeMeta: metav1.TypeMeta{ - Kind: "Role", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace", - Name: "csv-service-account--c1nyhtj4melkktv8nq58cczhkreg8wbfd2umv97vci", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - &rbacv1.RoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "RoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace", - Name: "csv-service-account--c1nyhtj4melkktv8nq58cczhkreg8wbfd2umv97vci", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "service-account-one", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "Role", - Name: "csv-service-account--c1nyhtj4melkktv8nq58cczhkreg8wbfd2umv97vci", - }, - }, - &rbacv1.Role{ - TypeMeta: metav1.TypeMeta{ - Kind: "Role", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace", - Name: "csv-service-account-d2x7x81lh02xpvfl0hrc7he83vd3svym7paq0oj39hk", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - &rbacv1.RoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "RoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace", - Name: "csv-service-account-d2x7x81lh02xpvfl0hrc7he83vd3svym7paq0oj39hk", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "service-account-two", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "Role", - Name: "csv-service-account-d2x7x81lh02xpvfl0hrc7he83vd3svym7paq0oj39hk", - }, - }, - }, - }, - { - name: "treats empty service account as 'default' service account", - installNamespace: "install-namespace", - targetNamespaces: []string{"watch-namespace"}, - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeAllNamespaces), - WithName("csv"), - WithPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - ), - ), - }, - expectedResources: []client.Object{ - &rbacv1.Role{ - TypeMeta: metav1.TypeMeta{ - Kind: "Role", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace", - Name: "csv-default-f0sf4spj31ti6476d21w12dcdo76i4alx2thty14vgc", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - &rbacv1.RoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "RoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: "watch-namespace", - Name: "csv-default-f0sf4spj31ti6476d21w12dcdo76i4alx2thty14vgc", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "default", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "Role", - Name: "csv-default-f0sf4spj31ti6476d21w12dcdo76i4alx2thty14vgc", - }, - }, - }, - }, - } { - t.Run(tc.name, func(t *testing.T) { - // ignore bundle validation for these unit tests as we only want to test - // the specific resource generation logic - conv := convert.PlainConverter - conv.BundleValidator = nil - plain, err := conv.Convert(tc.bundle, tc.installNamespace, tc.targetNamespaces) - require.NoError(t, err) - for _, expectedObj := range tc.expectedResources { - // find object in generated objects - result := filterutil.Filter(plain.Objects, byTargetObject(expectedObj)) - require.Len(t, result, 1) - require.Equal(t, expectedObj, result[0]) - } - }) - } -} - -func Test_Convert_ClusterRoleClusterRoleBindingResourceGeneration(t *testing.T) { - for _, tc := range []struct { - name string - installNamespace string - targetNamespaces []string - bundle render.RegistryV1 - expectedResources []client.Object - }{ - { - name: "promotes permissions to clusters permissions and adds namespace policy rule when in AllNamespaces mode (target namespace is [''])", - installNamespace: "install-namespace", - targetNamespaces: []string{""}, - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), - WithName("csv"), - WithPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-one", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-two", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - ), - ), - }, - expectedResources: []client.Object{ - &rbacv1.ClusterRole{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRole", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "csv-service-accoun-183oadrm9kfo7nconyoo014k1ff8dy5v3u5lbom19pat", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, { - Verbs: []string{"get", "list", "watch"}, - APIGroups: []string{corev1.GroupName}, - Resources: []string{"namespaces"}, - }, - }, - }, - &rbacv1.ClusterRoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "csv-service-accoun-183oadrm9kfo7nconyoo014k1ff8dy5v3u5lbom19pat", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "service-account-one", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "ClusterRole", - Name: "csv-service-accoun-183oadrm9kfo7nconyoo014k1ff8dy5v3u5lbom19pat", - }, - }, - &rbacv1.ClusterRole{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRole", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "csv-service-account-vgd0bghvdoibjpu1pj6maoju7rfr1odnhm2ylfxtfh3", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, { - Verbs: []string{"get", "list", "watch"}, - APIGroups: []string{corev1.GroupName}, - Resources: []string{"namespaces"}, - }, - }, - }, - &rbacv1.ClusterRoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "csv-service-account-vgd0bghvdoibjpu1pj6maoju7rfr1odnhm2ylfxtfh3", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "service-account-two", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "ClusterRole", - Name: "csv-service-account-vgd0bghvdoibjpu1pj6maoju7rfr1odnhm2ylfxtfh3", - }, - }, - }, - }, - { - name: "generates clusterroles and clusterrolebindings for clusterpermissions", - installNamespace: "install-namespace", - targetNamespaces: []string{"watch-namespace"}, - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), - WithName("csv"), - WithClusterPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-one", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-two", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - ), - ), - }, - expectedResources: []client.Object{ - &rbacv1.ClusterRole{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRole", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "csv-service-account--c1nyhtj4melkktv8nq58cczhkreg8wbfd2umv97vci", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - &rbacv1.ClusterRoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "csv-service-account--c1nyhtj4melkktv8nq58cczhkreg8wbfd2umv97vci", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "service-account-one", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "ClusterRole", - Name: "csv-service-account--c1nyhtj4melkktv8nq58cczhkreg8wbfd2umv97vci", - }, - }, - &rbacv1.ClusterRole{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRole", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "csv-service-account-d2x7x81lh02xpvfl0hrc7he83vd3svym7paq0oj39hk", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - &rbacv1.ClusterRoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "csv-service-account-d2x7x81lh02xpvfl0hrc7he83vd3svym7paq0oj39hk", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "service-account-two", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "ClusterRole", - Name: "csv-service-account-d2x7x81lh02xpvfl0hrc7he83vd3svym7paq0oj39hk", - }, - }, - }, - }, - { - name: "treats empty service accounts as 'default' service account", - installNamespace: "install-namespace", - targetNamespaces: []string{"watch-namespace"}, - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces, v1alpha1.InstallModeTypeSingleNamespace), - WithName("csv"), - WithClusterPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - ), - ), - }, - expectedResources: []client.Object{ - &rbacv1.ClusterRole{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRole", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "csv-default-f0sf4spj31ti6476d21w12dcdo76i4alx2thty14vgc", - }, - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - &rbacv1.ClusterRoleBinding{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRoleBinding", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "csv-default-f0sf4spj31ti6476d21w12dcdo76i4alx2thty14vgc", - }, - Subjects: []rbacv1.Subject{ - { - Kind: "ServiceAccount", - APIGroup: "", - Name: "default", - Namespace: "install-namespace", - }, - }, - RoleRef: rbacv1.RoleRef{ - APIGroup: "rbac.authorization.k8s.io", - Kind: "ClusterRole", - Name: "csv-default-f0sf4spj31ti6476d21w12dcdo76i4alx2thty14vgc", - }, - }, - }, - }, - } { - t.Run(tc.name, func(t *testing.T) { - // ignore bundle validation for these unit tests as we only want to test - // the specific resource generation logic - conv := convert.PlainConverter - conv.BundleValidator = nil - - plain, err := conv.Convert(tc.bundle, tc.installNamespace, tc.targetNamespaces) - require.NoError(t, err) - for _, expectedObj := range tc.expectedResources { - // find object in generated objects - result := filterutil.Filter(plain.Objects, byTargetObject(expectedObj)) - require.Len(t, result, 1) - require.Equal(t, expectedObj, result[0]) - } - }) - } -} - -func Test_Convert_ServiceAccountResourceGeneration(t *testing.T) { - for _, tc := range []struct { - name string - installNamespace string - targetNamespaces []string - bundle render.RegistryV1 - expectedResources []client.Object - }{ - { - name: "generates unique set of clusterpermissions and permissions service accounts in the install namespace", - installNamespace: "install-namespace", - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), - WithName("csv"), - WithPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-1", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-2", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - ), - WithClusterPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-2", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "service-account-3", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{"appsv1"}, - Resources: []string{"deployments"}, - Verbs: []string{"create"}, - }, - }, - }, - ), - ), - }, - expectedResources: []client.Object{ - &corev1.ServiceAccount{ - TypeMeta: metav1.TypeMeta{ - Kind: "ServiceAccount", - APIVersion: corev1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "service-account-1", - Namespace: "install-namespace", - }, - }, - &corev1.ServiceAccount{ - TypeMeta: metav1.TypeMeta{ - Kind: "ServiceAccount", - APIVersion: corev1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "service-account-2", - Namespace: "install-namespace", - }, - }, - &corev1.ServiceAccount{ - TypeMeta: metav1.TypeMeta{ - Kind: "ServiceAccount", - APIVersion: corev1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "service-account-3", - Namespace: "install-namespace", - }, - }, - }, - }, - { - name: "treats empty service accounts as default and doesn't generate them", - installNamespace: "install-namespace", - bundle: render.RegistryV1{ - CSV: MakeCSV( - WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces), - WithName("csv"), - WithPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - ), - WithClusterPermissions( - v1alpha1.StrategyDeploymentPermissions{ - ServiceAccountName: "", - Rules: []rbacv1.PolicyRule{ - { - APIGroups: []string{""}, - Resources: []string{"namespaces"}, - Verbs: []string{"get", "list", "watch"}, - }, - }, - }, - ), - ), - }, - expectedResources: nil, - }, - } { - t.Run(tc.name, func(t *testing.T) { - // ignore bundle validation for these unit tests as we only want to test - // the specific resource generation logic - conv := convert.PlainConverter - conv.BundleValidator = nil - plain, err := conv.Convert(tc.bundle, tc.installNamespace, tc.targetNamespaces) - require.NoError(t, err) - for _, expectedObj := range tc.expectedResources { - // find object in generated objects - result := filterutil.Filter(plain.Objects, byTargetObject(expectedObj)) - require.Len(t, result, 1) - require.Equal(t, expectedObj, result[0]) - } - }) - } -} - -func Test_Convert_BundleCRDGeneration(t *testing.T) { - bundle := render.RegistryV1{ - CRDs: []apiextensionsv1.CustomResourceDefinition{ - {ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}}, - {ObjectMeta: metav1.ObjectMeta{Name: "crd-two"}}, - }, - CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), - } - - // ignore bundle validation for these unit tests as we only want to test - // the specific resource generation logic - conv := convert.PlainConverter - conv.BundleValidator = nil - plain, err := conv.Convert(bundle, "install-namespace", []string{""}) - require.NoError(t, err) - expectedResources := []client.Object{ - &apiextensionsv1.CustomResourceDefinition{ObjectMeta: metav1.ObjectMeta{Name: "crd-one"}}, - &apiextensionsv1.CustomResourceDefinition{ObjectMeta: metav1.ObjectMeta{Name: "crd-two"}}, - } - - for _, expectedObj := range expectedResources { - // find object in generated objects - result := filterutil.Filter(plain.Objects, byTargetObject(expectedObj)) - require.Len(t, result, 1) - require.Equal(t, expectedObj, result[0]) - } -} - -func Test_Convert_AdditionalResourcesGeneration(t *testing.T) { - bundle := render.RegistryV1{ - CSV: MakeCSV(WithInstallModeSupportFor(v1alpha1.InstallModeTypeAllNamespaces)), - Others: []unstructured.Unstructured{ - convertToUnstructured(t, - &corev1.Service{ - TypeMeta: metav1.TypeMeta{ - Kind: "Service", - APIVersion: corev1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "bundled-service", - }, - }, - ), - convertToUnstructured(t, - &rbacv1.ClusterRole{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRole", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "bundled-clusterrole", - }, - }, - ), - }, - } - - // ignore bundle validation for these unit tests as we only want to test - // the specific resource generation logic - conv := convert.PlainConverter - conv.BundleValidator = nil - plain, err := conv.Convert(bundle, "install-namespace", []string{""}) - require.NoError(t, err) - expectedResources := []unstructured.Unstructured{ - convertToUnstructured(t, &corev1.Service{ - TypeMeta: metav1.TypeMeta{ - Kind: "Service", - APIVersion: corev1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "bundled-service", - Namespace: "install-namespace", - }, - }), - convertToUnstructured(t, &rbacv1.ClusterRole{ - TypeMeta: metav1.TypeMeta{ - Kind: "ClusterRole", - APIVersion: rbacv1.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "bundled-clusterrole", - }, - }), - } - - for _, expectedObj := range expectedResources { - // find object in generated objects - result := filterutil.Filter(plain.Objects, byTargetObject(&expectedObj)) - require.Len(t, result, 1) - require.Equal(t, &expectedObj, result[0]) - } -} - -func convertToUnstructured(t *testing.T, obj interface{}) unstructured.Unstructured { - unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&obj) - require.NoError(t, err) - require.NotNil(t, unstructuredObj) - return unstructured.Unstructured{Object: unstructuredObj} -} - func findObjectByName(name string, result []client.Object) client.Object { for _, o := range result { // Since this is a controlled env, comparing only the names is sufficient for now. @@ -1646,14 +401,6 @@ spec: } } -func byTargetObject(obj client.Object) filterutil.Predicate[client.Object] { - return func(entity client.Object) bool { - return entity.GetName() == obj.GetName() && - entity.GetNamespace() == obj.GetNamespace() && - entity.GetObjectKind().GroupVersionKind() == obj.GetObjectKind().GroupVersionKind() - } -} - func removePaths(mapFs fstest.MapFS, paths ...string) fstest.MapFS { for k := range mapFs { for _, path := range paths {