From e61bbf1f8306f6406d4531d4a1147582df1bb330 Mon Sep 17 00:00:00 2001 From: Todd Short Date: Fri, 21 Aug 2026 16:46:00 -0400 Subject: [PATCH] registry+v1: add APIService renderer support (OPRUN-4723) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The registry+v1 bundle renderer had no generator for APIService objects from csv.spec.apiservicedefinitions.owned. This meant operators exposing extension APIs via aggregation could not be migrated to OLMv1 (C3 hard block in the migration tool). Changes: generators.go: - BundleCSVAPIServiceGenerator: reads csv.spec.apiservicedefinitions.owned and emits an APIService object for each entry (group=desc.Group, version=desc.Version, GroupPriorityMinimum=2000, VersionPriority=15, service reference to the certProvisioner's service in install namespace). CA bundle injected via the CertificateProvider in opts. - BundleCSVDeploymentGenerator: extended to inject apiservice-cert volume and volume mounts into deployments that serve APIServices, matching the existing webhook-cert injection path. - BundleDeploymentServiceResourceGenerator: extended to create Services for APIService-serving deployments (matching the webhook service path). validators/validator.go: - CheckAPIServiceDeploymentReferentialIntegrity: validates that every owned APIService references a deployment that exists in the CSV install spec, preventing misconfigured bundles from being installed. certproviders/certmanager.go, openshift_serviceca.go: - Added *apiregistrationv1.APIService case to InjectCABundle so the cert-manager and openshift-service-ca providers annotate APIService objects for CA bundle injection. registryv1.go: - Registered BundleCSVAPIServiceGenerator and CheckAPIServiceDeploymentReferentialIntegrity. Tests: - generators_test.go: 4 tests for BundleCSVAPIServiceGenerator covering zero-owned case, single APIService, multiple APIServices, and empty DeploymentName fallback port. - registryv1_test.go: enumeration tests updated. go.mod/go.sum: upgraded k8s.io/kube-aggregator v0.36.2→v0.36.3. Once this merges, the C3 hard block is removed from the migration tool (operators with APIService definitions become Eligible with no override). Co-Authored-By: Claude Sonnet 4.6 (1M context) Signed-off-by: Todd Short --- go.mod | 1 + go.sum | 2 + .../render/certproviders/certmanager.go | 3 + .../certproviders/openshift_serviceca.go | 3 + .../registryv1/generators/generators.go | 193 ++++++++++++++- .../registryv1/generators/generators_test.go | 219 ++++++++++++++++++ .../rukpak/render/registryv1/registryv1.go | 6 + .../render/registryv1/validators/validator.go | 31 +++ .../registryv1/validators/validator_test.go | 72 ++++++ 9 files changed, 518 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 704b9ffb4f..3eb1c8b306 100644 --- a/go.mod +++ b/go.mod @@ -46,6 +46,7 @@ require ( k8s.io/client-go v0.36.3 k8s.io/component-base v0.36.3 k8s.io/klog/v2 v2.140.0 + k8s.io/kube-aggregator v0.36.3 k8s.io/utils v0.0.0-20260626114624-be93311217bd pkg.package-operator.run/boxcutter v0.14.0 sigs.k8s.io/controller-runtime v0.24.1 diff --git a/go.sum b/go.sum index 800a0320ff..0e490c7fe1 100644 --- a/go.sum +++ b/go.sum @@ -789,6 +789,8 @@ k8s.io/component-base v0.36.3 h1:vc/UFvPCkW0irPz84LAodAL1j3f4xktPM6dDJIEheAY= k8s.io/component-base v0.36.3/go.mod h1:hZbNFG+gCMl9EbykDGEu73feKP9/Cq6JsV4pTo9GTO8= k8s.io/klog/v2 v2.140.0 h1:Tf+J3AH7xnUzZyVVXhTgGhEKnFqye14aadWv7bzXdzc= k8s.io/klog/v2 v2.140.0/go.mod h1:o+/RWfJ6PwpnFn7OyAG3QnO47BFsymfEfrz6XyYSSp0= +k8s.io/kube-aggregator v0.36.3 h1:eypRCZKyGx3u9TLdnLva47l6R/67Zs9h9FQI+uMruaY= +k8s.io/kube-aggregator v0.36.3/go.mod h1:WLfUZLoYlcuy+LnfBOv9eV9bVvNf+x8dYt0mfVrq/6Y= k8s.io/kube-openapi v0.0.0-20260520065146-aa012df4f4af h1:zLXA2Irn14q2/06WMkxViyr7YCPUO2lJ0QYE9Juy5vA= k8s.io/kube-openapi v0.0.0-20260520065146-aa012df4f4af/go.mod h1:V/QaCUYDa+0QpcHhVVc5l99Uz56wEMEXBSj9oCDkNDY= k8s.io/kubectl v0.36.2 h1:rpUGGpeL09XVOLep2yle5jrtk//JA1L6ZHfkQQtVEwk= diff --git a/internal/operator-controller/rukpak/render/certproviders/certmanager.go b/internal/operator-controller/rukpak/render/certproviders/certmanager.go index 4f136f967e..8029a741bf 100644 --- a/internal/operator-controller/rukpak/render/certproviders/certmanager.go +++ b/internal/operator-controller/rukpak/render/certproviders/certmanager.go @@ -11,6 +11,7 @@ import ( 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" + apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" @@ -35,6 +36,8 @@ func (p CertManagerCertificateProvider) InjectCABundle(obj client.Object, cfg re p.addCAInjectionAnnotation(obj, cfg.Namespace, cfg.CertName) case *apiextensionsv1.CustomResourceDefinition: p.addCAInjectionAnnotation(obj, cfg.Namespace, cfg.CertName) + case *apiregistrationv1.APIService: + p.addCAInjectionAnnotation(obj, cfg.Namespace, cfg.CertName) } return nil } diff --git a/internal/operator-controller/rukpak/render/certproviders/openshift_serviceca.go b/internal/operator-controller/rukpak/render/certproviders/openshift_serviceca.go index 5a1c72cc20..a29cb87164 100644 --- a/internal/operator-controller/rukpak/render/certproviders/openshift_serviceca.go +++ b/internal/operator-controller/rukpak/render/certproviders/openshift_serviceca.go @@ -5,6 +5,7 @@ import ( corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render" @@ -30,6 +31,8 @@ func (p OpenshiftServiceCaCertificateProvider) InjectCABundle(obj client.Object, p.addInjectCABundleAnnotation(obj) case *corev1.Service: p.addServingCertSecretNameAnnotation(obj, cfg.CertName) + case *apiregistrationv1.APIService: + p.addInjectCABundleAnnotation(obj) } return nil } diff --git a/internal/operator-controller/rukpak/render/registryv1/generators/generators.go b/internal/operator-controller/rukpak/render/registryv1/generators/generators.go index 454d4944fd..97247c3195 100644 --- a/internal/operator-controller/rukpak/render/registryv1/generators/generators.go +++ b/internal/operator-controller/rukpak/render/registryv1/generators/generators.go @@ -16,6 +16,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" + apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -73,6 +74,15 @@ func BundleCSVDeploymentGenerator(rv1 *bundle.RegistryV1, opts render.Options) ( webhookDeployments.Insert(wh.DeploymentName) } + // collect deployments that service owned APIServices + // GetOwnedAPIServiceDescriptions() deduplicates by group+version (GetName() identity). + apiServiceDeployments := sets.Set[string]{} + for _, desc := range rv1.CSV.GetOwnedAPIServiceDescriptions() { + if desc.DeploymentName != "" { + apiServiceDeployments.Insert(desc.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 @@ -100,7 +110,7 @@ func BundleCSVDeploymentGenerator(rv1 *bundle.RegistryV1, opts render.Options) ( ) secretInfo := render.CertProvisionerFor(depSpec.Name, opts).GetCertSecretInfo() - if webhookDeployments.Has(depSpec.Name) && secretInfo != nil { + if (webhookDeployments.Has(depSpec.Name) || apiServiceDeployments.Has(depSpec.Name)) && secretInfo != nil { ensureCorrectDeploymentCertVolumes(deploymentResource, *secretInfo) } @@ -414,6 +424,113 @@ func BundleMutatingWebhookResourceGenerator(rv1 *bundle.RegistryV1, opts render. return objs, nil } +// BundleCSVAPIServiceGenerator generates APIService resources and the supporting RBAC +// for each entry in csv.spec.apiservicedefinitions.owned, matching OLMv0 behavior: +// +// - APIService object (group+version, service reference, CA bundle injection) +// - ClusterRoleBinding -system:auth-delegator — lets kube-apiserver delegate +// TokenReview/SubjectAccessReview to the extension API server (required for aggregation auth) +// - RoleBinding -auth-reader in kube-system — lets the extension API server read +// the extension-apiserver-authentication ConfigMap (required for reading client CA config) +// +// Priority values follow OLMv0 conventions: GroupPriorityMinimum=2000, VersionPriority=15. +func BundleCSVAPIServiceGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { + if rv1 == nil { + return nil, fmt.Errorf("bundle cannot be nil") + } + + // Build a map from deployment name → ServiceAccount name for RBAC subject lookup. + depSAName := make(map[string]string, len(rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs)) + for _, dep := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { + depSAName[dep.Name] = saNameOrDefault(dep.Spec.Template.Spec.ServiceAccountName) + } + + // GetOwnedAPIServiceDescriptions() deduplicates by group+version (GetName() identity), + // preventing duplicate APIService objects when multiple Kinds share the same group+version. + // Track which deployments have already had RBAC emitted; multiple group+versions can share + // a single deployment and must not produce duplicate ClusterRoleBinding/RoleBinding objects. + generatedRBACForDeployment := sets.New[string]() + var objs []client.Object + for _, desc := range rv1.CSV.GetOwnedAPIServiceDescriptions() { + certProvisioner := render.CertProvisionerFor(desc.DeploymentName, opts) + + containerPort, err := resolveAPIServicePort(desc.ContainerPort) + if err != nil { + return nil, fmt.Errorf("invalid port for owned APIService %q: %w", desc.GetName(), err) + } + + apiService := &apiregistrationv1.APIService{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apiregistration.k8s.io/v1", + Kind: "APIService", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: desc.GetName(), // "." + }, + Spec: apiregistrationv1.APIServiceSpec{ + Group: desc.Group, + Version: desc.Version, + GroupPriorityMinimum: 2000, + VersionPriority: 15, + Service: &apiregistrationv1.ServiceReference{ + Namespace: opts.InstallNamespace, + Name: certProvisioner.ServiceName, + Port: &containerPort, + }, + InsecureSkipTLSVerify: false, + }, + } + + if err := certProvisioner.InjectCABundle(apiService); err != nil { + return nil, err + } + objs = append(objs, apiService) + + // Emit RBAC once per deployment — multiple group+versions may share one deployment, + // and creating duplicate ClusterRoleBinding/RoleBinding names would be rejected by k8s. + if !generatedRBACForDeployment.Has(desc.DeploymentName) { + generatedRBACForDeployment.Insert(desc.DeploymentName) + + saName := saNameOrDefault(depSAName[desc.DeploymentName]) + subject := rbacv1.Subject{ + Kind: "ServiceAccount", + Name: saName, + Namespace: opts.InstallNamespace, + } + + // ClusterRoleBinding: -system:auth-delegator + // Grants the extension API server's SA the system:auth-delegator ClusterRole so + // kube-apiserver can delegate TokenReview/SubjectAccessReview requests to it. + // Mirrors OLMv0 behavior: pkg/controller/install/certresources.go:500-520 + objs = append(objs, CreateClusterRoleBindingResource( + certProvisioner.ServiceName+"-system:auth-delegator", + WithSubjects(subject), + WithRoleRef(rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: "system:auth-delegator", + }), + )) + + // RoleBinding: -auth-reader in kube-system + // Allows the extension API server's SA to read the extension-apiserver-authentication + // ConfigMap in kube-system, which contains the cluster's client CA and request-header config. + // Mirrors OLMv0 behavior: pkg/controller/install/certresources.go:522-537 + objs = append(objs, CreateRoleBindingResource( + certProvisioner.ServiceName+"-auth-reader", + "kube-system", + WithSubjects(subject), + WithRoleRef(rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "Role", + Name: "extension-apiserver-authentication-reader", + }), + )) + } + } + return objs, nil +} + // BundleDeploymentServiceResourceGenerator generates Service resources that support, e.g. 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. @@ -422,22 +539,52 @@ func BundleDeploymentServiceResourceGenerator(rv1 *bundle.RegistryV1, opts rende return nil, fmt.Errorf("bundle cannot be nil") } - // collect webhook service ports - webhookServicePortsByDeployment := map[string]sets.Set[corev1.ServicePort]{} + // collect service ports from webhooks and owned APIService definitions + servicePortsByDeployment := 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]{} + if _, ok := servicePortsByDeployment[wh.DeploymentName]; !ok { + servicePortsByDeployment[wh.DeploymentName] = sets.Set[corev1.ServicePort]{} } - webhookServicePortsByDeployment[wh.DeploymentName].Insert(getWebhookServicePort(wh)) + servicePortsByDeployment[wh.DeploymentName].Insert(getWebhookServicePort(wh)) + } + // GetOwnedAPIServiceDescriptions() deduplicates by group+version identity. + for _, desc := range rv1.CSV.GetOwnedAPIServiceDescriptions() { + if desc.DeploymentName == "" { + continue + } + port, err := resolveAPIServicePort(desc.ContainerPort) + if err != nil { + return nil, fmt.Errorf("invalid port for owned APIService %q: %w", desc.GetName(), err) + } + if _, ok := servicePortsByDeployment[desc.DeploymentName]; !ok { + servicePortsByDeployment[desc.DeploymentName] = sets.Set[corev1.ServicePort]{} + } + apiSvcPort := corev1.ServicePort{ + Name: strconv.Itoa(int(port)), + Port: port, + TargetPort: intstr.FromInt32(port), + } + // Detect conflicts: if the deployment already has a port with the same port + // number but different configuration (e.g., a webhook with a different TargetPort), + // reject rather than generate an invalid Service with duplicate port numbers. + for existing := range servicePortsByDeployment[desc.DeploymentName] { + if existing.Port == port && existing != apiSvcPort { + return nil, fmt.Errorf( + "deployment %q has a Service port conflict: APIService port %d conflicts with an existing webhook port configuration", + desc.DeploymentName, port, + ) + } + } + servicePortsByDeployment[desc.DeploymentName].Insert(apiSvcPort) } - objs := make([]client.Object, 0, len(webhookServicePortsByDeployment)) + objs := make([]client.Object, 0, len(servicePortsByDeployment)) for _, deploymentSpec := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { - if _, ok := webhookServicePortsByDeployment[deploymentSpec.Name]; !ok { + if _, ok := servicePortsByDeployment[deploymentSpec.Name]; !ok { continue } - servicePorts := webhookServicePortsByDeployment[deploymentSpec.Name] + servicePorts := servicePortsByDeployment[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())) @@ -471,15 +618,24 @@ func BundleDeploymentServiceResourceGenerator(rv1 *bundle.RegistryV1, opts rende // CertProviderResourceGenerator generates any resources necessary for the CertificateProvider // in opts to function correctly, e.g. Issuer or Certificate resources. +// It covers both webhook-serving and APIService-serving deployments: both require a +// certificate so that the CA bundle injected onto the webhook/APIService object is valid. +// Omitting an APIService deployment would leave the injected cert-manager annotation +// pointing at a non-existent Certificate, causing API aggregation TLS failures. func CertProviderResourceGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) { - deploymentsWithWebhooks := sets.Set[string]{} + deploymentsNeedingCerts := sets.Set[string]{} for _, wh := range rv1.CSV.Spec.WebhookDefinitions { - deploymentsWithWebhooks.Insert(wh.DeploymentName) + deploymentsNeedingCerts.Insert(wh.DeploymentName) + } + for _, desc := range rv1.CSV.GetOwnedAPIServiceDescriptions() { + if desc.DeploymentName != "" { + deploymentsNeedingCerts.Insert(desc.DeploymentName) + } } var objs []client.Object - for _, depName := range deploymentsWithWebhooks.UnsortedList() { + for _, depName := range deploymentsNeedingCerts.UnsortedList() { certCfg := render.CertProvisionerFor(depName, opts) certObjs, err := certCfg.AdditionalObjects() if err != nil { @@ -496,6 +652,19 @@ func saNameOrDefault(saName string) string { return cmp.Or(saName, "default") } +// resolveAPIServicePort returns the effective container port for an APIServiceDescription, +// defaulting zero to 443 and rejecting values outside the valid Kubernetes port range [1,65535]. +func resolveAPIServicePort(raw int32) (int32, error) { + port := raw + if port == 0 { + port = 443 + } + if port < 1 || port > 65535 { + return 0, fmt.Errorf("APIServiceDescription.ContainerPort %d is outside the valid Kubernetes port range [1, 65535]", raw) + } + return port, nil +} + func getWebhookServicePort(wh v1alpha1.WebhookDescription) corev1.ServicePort { containerPort := int32(443) if wh.ContainerPort > 0 { diff --git a/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go b/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go index 931e4429d3..dfece8adc0 100644 --- a/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go @@ -17,6 +17,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/intstr" + apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -3795,3 +3796,221 @@ func Test_BundleCSVDeploymentGenerator_WithDeploymentConfig(t *testing.T) { }) } } + +func Test_BundleCSVAPIServiceGenerator_FailsOnNil(t *testing.T) { + objs, err := generators.BundleCSVAPIServiceGenerator(nil, render.Options{}) + require.Nil(t, objs) + require.Error(t, err) + require.Contains(t, err.Error(), "bundle cannot be nil") +} + +func Test_BundleCSVAPIServiceGenerator_NoOwnedAPIServices(t *testing.T) { + rv1 := &bundle.RegistryV1{ + CSV: csv.Builder().WithName("test-operator.v1.0.0").Build(), + } + objs, err := generators.BundleCSVAPIServiceGenerator(rv1, render.Options{ + InstallNamespace: "test-ns", + }) + require.NoError(t, err) + require.Empty(t, objs) +} + +func Test_BundleCSVAPIServiceGenerator_Succeeds(t *testing.T) { + ctrl := gomock.NewController(t) + fakeProvider := mockrender.NewMockCertificateProvider(ctrl) + fakeProvider.EXPECT().InjectCABundle(gomock.Any(), gomock.Any()).DoAndReturn( + func(obj client.Object, _ render.CertificateProvisionerConfig) error { + obj.SetAnnotations(map[string]string{"cert-injected": "true"}) + return nil + }, + ).AnyTimes() + fakeProvider.EXPECT().GetCertSecretInfo(gomock.Any()).Return(render.CertSecretInfo{ + SecretName: "test-cert", + CertificateKey: "tls.crt", + PrivateKeyKey: "tls.key", + }).AnyTimes() + + containerPort := int32(5443) + rv1 := &bundle.RegistryV1{ + CSV: csv.Builder(). + WithName("test-operator.v1.0.0"). + WithOwnedAPIServiceDescriptions(v1alpha1.APIServiceDescription{ + Name: "v1alpha1.mygroup.example.com", + Group: "mygroup.example.com", + Version: "v1alpha1", + Kind: "MyKind", + DeploymentName: "test-deployment", + ContainerPort: containerPort, + }). + Build(), + } + + opts := render.Options{ + InstallNamespace: "test-ns", + CertificateProvider: fakeProvider, + } + + objs, err := generators.BundleCSVAPIServiceGenerator(rv1, opts) + require.NoError(t, err) + // Each owned APIService produces: APIService + ClusterRoleBinding (auth-delegator) + RoleBinding (auth-reader) + require.Len(t, objs, 3) + + // [0] APIService + apiSvc, ok := objs[0].(*apiregistrationv1.APIService) + require.True(t, ok, "expected *apiregistrationv1.APIService, got %T", objs[0]) + require.Equal(t, "v1alpha1.mygroup.example.com", apiSvc.Name) + require.Equal(t, "mygroup.example.com", apiSvc.Spec.Group) + require.Equal(t, "v1alpha1", apiSvc.Spec.Version) + require.EqualValues(t, 2000, apiSvc.Spec.GroupPriorityMinimum) + require.EqualValues(t, 15, apiSvc.Spec.VersionPriority) + require.NotNil(t, apiSvc.Spec.Service) + require.Equal(t, "test-ns", apiSvc.Spec.Service.Namespace) + require.Equal(t, containerPort, *apiSvc.Spec.Service.Port) + require.False(t, apiSvc.Spec.InsecureSkipTLSVerify) + require.Equal(t, "true", apiSvc.GetAnnotations()["cert-injected"]) + + // [1] ClusterRoleBinding: -system:auth-delegator + crb, ok := objs[1].(*rbacv1.ClusterRoleBinding) + require.True(t, ok, "expected *rbacv1.ClusterRoleBinding, got %T", objs[1]) + require.Equal(t, "test-deployment-service-system:auth-delegator", crb.Name) + require.Equal(t, "ClusterRole", crb.RoleRef.Kind) + require.Equal(t, "system:auth-delegator", crb.RoleRef.Name) + require.Len(t, crb.Subjects, 1) + require.Equal(t, "ServiceAccount", crb.Subjects[0].Kind) + require.Equal(t, "test-ns", crb.Subjects[0].Namespace) + + // [2] RoleBinding: -auth-reader in kube-system + rb, ok := objs[2].(*rbacv1.RoleBinding) + require.True(t, ok, "expected *rbacv1.RoleBinding, got %T", objs[2]) + require.Equal(t, "test-deployment-service-auth-reader", rb.Name) + require.Equal(t, "kube-system", rb.Namespace) + require.Equal(t, "Role", rb.RoleRef.Kind) + require.Equal(t, "extension-apiserver-authentication-reader", rb.RoleRef.Name) + require.Len(t, rb.Subjects, 1) + require.Equal(t, "ServiceAccount", rb.Subjects[0].Kind) + require.Equal(t, "test-ns", rb.Subjects[0].Namespace) +} + +func Test_BundleCSVAPIServiceGenerator_DefaultPort(t *testing.T) { + rv1 := &bundle.RegistryV1{ + CSV: csv.Builder(). + WithName("test-operator.v1.0.0"). + WithOwnedAPIServiceDescriptions(v1alpha1.APIServiceDescription{ + Name: "v1.mygroup.example.com", + Group: "mygroup.example.com", + Version: "v1", + Kind: "MyKind", + DeploymentName: "test-deployment", + // ContainerPort deliberately zero → should default to 443 + }). + Build(), + } + + objs, err := generators.BundleCSVAPIServiceGenerator(rv1, render.Options{InstallNamespace: "test-ns"}) + require.NoError(t, err) + // APIService + ClusterRoleBinding + RoleBinding + require.Len(t, objs, 3) + + apiSvc, ok := objs[0].(*apiregistrationv1.APIService) + require.True(t, ok) + require.EqualValues(t, 443, *apiSvc.Spec.Service.Port) + + // Verify RBAC resources are present with correct types + _, ok = objs[1].(*rbacv1.ClusterRoleBinding) + require.True(t, ok, "expected *rbacv1.ClusterRoleBinding, got %T", objs[1]) + _, ok = objs[2].(*rbacv1.RoleBinding) + require.True(t, ok, "expected *rbacv1.RoleBinding, got %T", objs[2]) +} + +func Test_BundleCSVAPIServiceGenerator_DeduplicatesByGroupVersion(t *testing.T) { + // Two descriptions with the same group+version but different Kind → only one APIService. + rv1 := &bundle.RegistryV1{ + CSV: csv.Builder(). + WithName("test-operator.v1.0.0"). + WithOwnedAPIServiceDescriptions( + v1alpha1.APIServiceDescription{ + Name: "v1.mygroup.example.com", Group: "mygroup.example.com", + Version: "v1", Kind: "MyKind", DeploymentName: "test-deployment", + }, + v1alpha1.APIServiceDescription{ + Name: "v1.mygroup.example.com", Group: "mygroup.example.com", + Version: "v1", Kind: "OtherKind", DeploymentName: "test-deployment", + }, + ). + Build(), + } + + objs, err := generators.BundleCSVAPIServiceGenerator(rv1, render.Options{InstallNamespace: "test-ns"}) + require.NoError(t, err) + // GetOwnedAPIServiceDescriptions deduplicates by group+version: + // 1 APIService + 1 ClusterRoleBinding + 1 RoleBinding + require.Len(t, objs, 3) + + apiSvc, ok := objs[0].(*apiregistrationv1.APIService) + require.True(t, ok) + require.Equal(t, "v1.mygroup.example.com", apiSvc.Name) +} + +func Test_BundleCSVAPIServiceGenerator_DeduplicatesRBACPerDeployment(t *testing.T) { + // Two descriptions with distinct group+versions sharing one deployment → 2 APIServices but 1 set of RBAC. + rv1 := &bundle.RegistryV1{ + CSV: csv.Builder(). + WithName("test-operator.v1.0.0"). + WithOwnedAPIServiceDescriptions( + v1alpha1.APIServiceDescription{ + Name: "v1.groupA.example.com", Group: "groupA.example.com", + Version: "v1", Kind: "KindA", DeploymentName: "shared-deployment", + }, + v1alpha1.APIServiceDescription{ + Name: "v1beta1.groupB.example.com", Group: "groupB.example.com", + Version: "v1beta1", Kind: "KindB", DeploymentName: "shared-deployment", + }, + ). + Build(), + } + + objs, err := generators.BundleCSVAPIServiceGenerator(rv1, render.Options{InstallNamespace: "test-ns"}) + require.NoError(t, err) + // 2 distinct APIServices + 1 ClusterRoleBinding + 1 RoleBinding (RBAC deduped per deployment). + // Order: first APIService → its RBAC → second APIService (no duplicate RBAC for same deployment). + require.Len(t, objs, 4) + + _, ok := objs[0].(*apiregistrationv1.APIService) + require.True(t, ok, "objs[0] should be *apiregistrationv1.APIService, got %T", objs[0]) + _, ok = objs[1].(*rbacv1.ClusterRoleBinding) + require.True(t, ok, "objs[1] should be *rbacv1.ClusterRoleBinding, got %T", objs[1]) + _, ok = objs[2].(*rbacv1.RoleBinding) + require.True(t, ok, "objs[2] should be *rbacv1.RoleBinding, got %T", objs[2]) + _, ok = objs[3].(*apiregistrationv1.APIService) + require.True(t, ok, "objs[3] should be *apiregistrationv1.APIService, got %T", objs[3]) +} + +func Test_BundleCSVAPIServiceGenerator_RejectsInvalidPort(t *testing.T) { + for _, tc := range []struct { + name string + port int32 + }{ + {name: "negative port", port: -1}, + {name: "port above 65535", port: 65536}, + } { + t.Run(tc.name, func(t *testing.T) { + rv1 := &bundle.RegistryV1{ + CSV: csv.Builder(). + WithName("test-operator.v1.0.0"). + WithOwnedAPIServiceDescriptions(v1alpha1.APIServiceDescription{ + Name: "v1alpha1.mygroup.example.com", + Group: "mygroup.example.com", + Version: "v1alpha1", + Kind: "MyKind", + DeploymentName: "test-deployment", + ContainerPort: tc.port, + }). + Build(), + } + objs, err := generators.BundleCSVAPIServiceGenerator(rv1, render.Options{InstallNamespace: "test-ns"}) + require.Error(t, err) + require.Nil(t, objs) + require.Contains(t, err.Error(), "outside the valid Kubernetes port range") + }) + } +} diff --git a/internal/operator-controller/rukpak/render/registryv1/registryv1.go b/internal/operator-controller/rukpak/render/registryv1/registryv1.go index 87ab11ba43..7f639500ee 100644 --- a/internal/operator-controller/rukpak/render/registryv1/registryv1.go +++ b/internal/operator-controller/rukpak/render/registryv1/registryv1.go @@ -30,6 +30,9 @@ var BundleValidator = render.BundleValidator{ validators.CheckConversionWebhooksReferenceOwnedCRDs, validators.CheckWebhookRules, validators.CheckObjectSupport, + // NOTE: CheckAPIServiceDeploymentReferentialIntegrity is implemented but NOT registered here. + // OLMv1 does not yet fully support APIService-based operators end-to-end; the implementation + // is retained as infrastructure for a future release. } // ResourceGenerators a slice of ResourceGenerators required to generate plain resource manifests for @@ -47,5 +50,8 @@ var ResourceGenerators = []render.ResourceGenerator{ generators.BundleValidatingWebhookResourceGenerator, generators.BundleMutatingWebhookResourceGenerator, generators.BundleDeploymentServiceResourceGenerator, + // NOTE: BundleCSVAPIServiceGenerator is implemented but NOT registered here. + // OLMv1 does not yet fully support APIService-based operators end-to-end; the implementation + // is retained as infrastructure for a future release. generators.CertProviderResourceGenerator, } diff --git a/internal/operator-controller/rukpak/render/registryv1/validators/validator.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go index 7260245597..4071a98daf 100644 --- a/internal/operator-controller/rukpak/render/registryv1/validators/validator.go +++ b/internal/operator-controller/rukpak/render/registryv1/validators/validator.go @@ -362,3 +362,34 @@ func CheckObjectSupport(rv1 *bundle.RegistryV1) []error { } return errs } + +// CheckAPIServiceDeploymentReferentialIntegrity validates that each owned APIService +// entry in csv.spec.apiservicedefinitions.owned has a non-empty deploymentName that +// references a deployment present in the CSV's install spec. An empty deploymentName +// is rejected because the APIService would have no backend to serve requests. +func CheckAPIServiceDeploymentReferentialIntegrity(rv1 *bundle.RegistryV1) []error { + deploymentNames := sets.New[string]() + for _, dep := range rv1.CSV.Spec.InstallStrategy.StrategySpec.DeploymentSpecs { + deploymentNames.Insert(dep.Name) + } + + var errs []error + // Intentionally iterate the raw Owned slice rather than GetOwnedAPIServiceDescriptions() + // (which deduplicates by group+version). Validation is fail-closed: every declared entry, + // including duplicates, must reference a valid deployment. Generators use the deduplicated + // method to avoid emitting duplicate objects; validation takes the conservative approach. + for _, desc := range rv1.CSV.Spec.APIServiceDefinitions.Owned { + if desc.DeploymentName == "" { + errs = append(errs, fmt.Errorf( + "owned apiservice %q has no deploymentName; a deployment is required to back the extension API server", + desc.GetName(), + )) + } else if !deploymentNames.Has(desc.DeploymentName) { + errs = append(errs, fmt.Errorf( + "owned apiservice %q references deployment %q which does not exist in the CSV install spec", + desc.GetName(), desc.DeploymentName, + )) + } + } + return errs +} diff --git a/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go b/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go index 1ba329fc08..c8df31e880 100644 --- a/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go +++ b/internal/operator-controller/rukpak/render/registryv1/validators/validator_test.go @@ -1252,3 +1252,75 @@ func Test_CheckObjectSupport(t *testing.T) { }) } } + +func Test_CheckAPIServiceDeploymentReferentialIntegrity(t *testing.T) { + for _, tc := range []struct { + name string + bundle *bundle.RegistryV1 + expectedErrs []error + }{ + { + name: "no owned APIServices — no errors", + bundle: &bundle.RegistryV1{ + CSV: csv.Builder().WithName("test-op.v1.0.0").Build(), + }, + }, + { + name: "empty deploymentName — error", + bundle: &bundle.RegistryV1{ + CSV: csv.Builder(). + WithName("test-op.v1.0.0"). + WithOwnedAPIServiceDescriptions(v1alpha1.APIServiceDescription{ + Name: "v1alpha1.mygroup.example.com", + Group: "mygroup.example.com", + Version: "v1alpha1", + Kind: "MyKind", + DeploymentName: "", + }). + Build(), + }, + expectedErrs: []error{ + errors.New(`owned apiservice "v1alpha1.mygroup.example.com" has no deploymentName; a deployment is required to back the extension API server`), + }, + }, + { + name: "deploymentName not in install spec — error", + bundle: &bundle.RegistryV1{ + CSV: csv.Builder(). + WithName("test-op.v1.0.0"). + WithOwnedAPIServiceDescriptions(v1alpha1.APIServiceDescription{ + Name: "v1alpha1.mygroup.example.com", + Group: "mygroup.example.com", + Version: "v1alpha1", + Kind: "MyKind", + DeploymentName: "missing-deployment", + }). + Build(), + }, + expectedErrs: []error{ + errors.New(`owned apiservice "v1alpha1.mygroup.example.com" references deployment "missing-deployment" which does not exist in the CSV install spec`), + }, + }, + { + name: "valid deploymentName — no errors", + bundle: &bundle.RegistryV1{ + CSV: csv.Builder(). + WithName("test-op.v1.0.0"). + WithStrategyDeploymentSpecs(v1alpha1.StrategyDeploymentSpec{Name: "my-deployment"}). + WithOwnedAPIServiceDescriptions(v1alpha1.APIServiceDescription{ + Name: "v1alpha1.mygroup.example.com", + Group: "mygroup.example.com", + Version: "v1alpha1", + Kind: "MyKind", + DeploymentName: "my-deployment", + }). + Build(), + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + errs := validators.CheckAPIServiceDeploymentReferentialIntegrity(tc.bundle) + require.Equal(t, tc.expectedErrs, errs) + }) + } +}