Skip to content

Commit ac14a4e

Browse files
perdasilvaPer Goncalves da Silva
perdasilva
and
Per Goncalves da Silva
authored
🌱 Bundle renderer refactor webhook service related function and attribute naming (#1999)
* Refactor certprovider WebhookServiceName to ServiceName Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> * Refactor BundleWebhookServiceResourceGenerator to BundleDeploymentServiceResourceGenerator Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> --------- Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> Co-authored-by: Per Goncalves da Silva <pegoncal@redhat.com>
1 parent 52b1265 commit ac14a4e

File tree

9 files changed

+78
-78
lines changed

9 files changed

+78
-78
lines changed

‎internal/operator-controller/rukpak/render/certprovider.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ type CertSecretInfo struct {
2828
// CertificateProvisionerConfig contains the necessary information for a CertificateProvider
2929
// to correctly generate and modify object for certificate injection and automation
3030
type CertificateProvisionerConfig struct {
31-
WebhookServiceName string
32-
CertName string
33-
Namespace string
34-
CertProvider CertificateProvider
31+
ServiceName string
32+
CertName string
33+
Namespace string
34+
CertProvider CertificateProvider
3535
}
3636

3737
// CertificateProvisioner uses a CertificateProvider to modify and generate objects based on its
@@ -70,9 +70,9 @@ func CertProvisionerFor(deploymentName string, opts Options) CertificateProvisio
7070
certName := util.ObjectNameForBaseAndSuffix(webhookServiceName, "cert")
7171

7272
return CertificateProvisioner{
73-
CertProvider: opts.CertificateProvider,
74-
WebhookServiceName: webhookServiceName,
75-
Namespace: opts.InstallNamespace,
76-
CertName: certName,
73+
CertProvider: opts.CertificateProvider,
74+
ServiceName: webhookServiceName,
75+
Namespace: opts.InstallNamespace,
76+
CertName: certName,
7777
}
7878
}

‎internal/operator-controller/rukpak/render/certprovider_test.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ import (
1616

1717
func Test_CertificateProvisioner_WithoutCertProvider(t *testing.T) {
1818
provisioner := &render.CertificateProvisioner{
19-
WebhookServiceName: "webhook",
20-
CertName: "cert",
21-
Namespace: "namespace",
22-
CertProvider: nil,
19+
ServiceName: "webhook",
20+
CertName: "cert",
21+
Namespace: "namespace",
22+
CertProvider: nil,
2323
}
2424

2525
require.NoError(t, provisioner.InjectCABundle(&corev1.Secret{}))
@@ -50,10 +50,10 @@ func Test_CertificateProvisioner_WithCertProvider(t *testing.T) {
5050
},
5151
}
5252
provisioner := &render.CertificateProvisioner{
53-
WebhookServiceName: "webhook",
54-
CertName: "cert",
55-
Namespace: "namespace",
56-
CertProvider: fakeProvider,
53+
ServiceName: "webhook",
54+
CertName: "cert",
55+
Namespace: "namespace",
56+
CertProvider: fakeProvider,
5757
}
5858

5959
svc := &corev1.Service{}
@@ -83,10 +83,10 @@ func Test_CertificateProvisioner_Errors(t *testing.T) {
8383
},
8484
}
8585
provisioner := &render.CertificateProvisioner{
86-
WebhookServiceName: "webhook",
87-
CertName: "cert",
88-
Namespace: "namespace",
89-
CertProvider: fakeProvider,
86+
ServiceName: "webhook",
87+
CertName: "cert",
88+
Namespace: "namespace",
89+
CertProvider: fakeProvider,
9090
}
9191

9292
err := provisioner.InjectCABundle(&corev1.Service{})
@@ -107,16 +107,16 @@ func Test_CertProvisionerFor(t *testing.T) {
107107
})
108108

109109
require.Equal(t, prov.CertProvider, fakeProvider)
110-
require.Equal(t, "my-deployment-thing-service", prov.WebhookServiceName)
110+
require.Equal(t, "my-deployment-thing-service", prov.ServiceName)
111111
require.Equal(t, "my-deployment-thing-service-cert", prov.CertName)
112112
require.Equal(t, "my-namespace", prov.Namespace)
113113
}
114114

115115
func Test_CertProvisionerFor_ExtraLargeName_MoreThan63Chars(t *testing.T) {
116116
prov := render.CertProvisionerFor("my.object.thing.has.a.really.really.really.really.really.long.name", render.Options{})
117117

118-
require.Len(t, prov.WebhookServiceName, 63)
118+
require.Len(t, prov.ServiceName, 63)
119119
require.Len(t, prov.CertName, 63)
120-
require.Equal(t, "my-object-thing-has-a-really-really-really-really-reall-service", prov.WebhookServiceName)
120+
require.Equal(t, "my-object-thing-has-a-really-really-really-really-reall-service", prov.ServiceName)
121121
require.Equal(t, "my-object-thing-has-a-really-really-really-really-reall-se-cert", prov.CertName)
122122
}

‎internal/operator-controller/rukpak/render/certproviders/certmanager.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,13 +153,13 @@ func (p CertManagerCertificateProvider) AdditionalObjects(cfg render.Certificate
153153
},
154154
Spec: certmanagerv1.CertificateSpec{
155155
SecretName: cfg.CertName,
156-
CommonName: fmt.Sprintf("%s.%s", cfg.WebhookServiceName, cfg.Namespace),
156+
CommonName: fmt.Sprintf("%s.%s", cfg.ServiceName, cfg.Namespace),
157157
Usages: []certmanagerv1.KeyUsage{certmanagerv1.UsageServerAuth},
158158
IsCA: false,
159159
DNSNames: []string{
160-
fmt.Sprintf("%s.%s", cfg.WebhookServiceName, cfg.Namespace),
161-
fmt.Sprintf("%s.%s.svc", cfg.WebhookServiceName, cfg.Namespace),
162-
fmt.Sprintf("%s.%s.svc.cluster.local", cfg.WebhookServiceName, cfg.Namespace),
160+
fmt.Sprintf("%s.%s", cfg.ServiceName, cfg.Namespace),
161+
fmt.Sprintf("%s.%s.svc", cfg.ServiceName, cfg.Namespace),
162+
fmt.Sprintf("%s.%s.svc.cluster.local", cfg.ServiceName, cfg.Namespace),
163163
},
164164
IssuerRef: certmanagermetav1.ObjectReference{
165165
Name: issuer.GetName(),

‎internal/operator-controller/rukpak/render/certproviders/certmanager_test.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ func Test_CertManagerProvider_InjectCABundle(t *testing.T) {
3030
name: "injects certificate annotation in validating webhook configuration",
3131
obj: &admissionregistrationv1.ValidatingWebhookConfiguration{},
3232
cfg: render.CertificateProvisionerConfig{
33-
WebhookServiceName: "webhook-service",
34-
Namespace: "namespace",
35-
CertName: "cert-name",
33+
ServiceName: "webhook-service",
34+
Namespace: "namespace",
35+
CertName: "cert-name",
3636
},
3737
expectedObj: &admissionregistrationv1.ValidatingWebhookConfiguration{
3838
ObjectMeta: metav1.ObjectMeta{
@@ -46,9 +46,9 @@ func Test_CertManagerProvider_InjectCABundle(t *testing.T) {
4646
name: "injects certificate annotation in mutating webhook configuration",
4747
obj: &admissionregistrationv1.MutatingWebhookConfiguration{},
4848
cfg: render.CertificateProvisionerConfig{
49-
WebhookServiceName: "webhook-service",
50-
Namespace: "namespace",
51-
CertName: "cert-name",
49+
ServiceName: "webhook-service",
50+
Namespace: "namespace",
51+
CertName: "cert-name",
5252
},
5353
expectedObj: &admissionregistrationv1.MutatingWebhookConfiguration{
5454
ObjectMeta: metav1.ObjectMeta{
@@ -62,9 +62,9 @@ func Test_CertManagerProvider_InjectCABundle(t *testing.T) {
6262
name: "injects certificate annotation in custom resource definition",
6363
obj: &apiextensionsv1.CustomResourceDefinition{},
6464
cfg: render.CertificateProvisionerConfig{
65-
WebhookServiceName: "webhook-service",
66-
Namespace: "namespace",
67-
CertName: "cert-name",
65+
ServiceName: "webhook-service",
66+
Namespace: "namespace",
67+
CertName: "cert-name",
6868
},
6969
expectedObj: &apiextensionsv1.CustomResourceDefinition{
7070
ObjectMeta: metav1.ObjectMeta{
@@ -78,9 +78,9 @@ func Test_CertManagerProvider_InjectCABundle(t *testing.T) {
7878
name: "ignores other objects",
7979
obj: &corev1.Service{},
8080
cfg: render.CertificateProvisionerConfig{
81-
WebhookServiceName: "webhook-service",
82-
Namespace: "namespace",
83-
CertName: "cert-name",
81+
ServiceName: "webhook-service",
82+
Namespace: "namespace",
83+
CertName: "cert-name",
8484
},
8585
expectedObj: &corev1.Service{},
8686
},
@@ -96,9 +96,9 @@ func Test_CertManagerProvider_InjectCABundle(t *testing.T) {
9696
func Test_CertManagerProvider_AdditionalObjects(t *testing.T) {
9797
certProvier := certproviders.CertManagerCertificateProvider{}
9898
objs, err := certProvier.AdditionalObjects(render.CertificateProvisionerConfig{
99-
WebhookServiceName: "webhook-service",
100-
Namespace: "namespace",
101-
CertName: "cert-name",
99+
ServiceName: "webhook-service",
100+
Namespace: "namespace",
101+
CertName: "cert-name",
102102
})
103103
require.NoError(t, err)
104104
require.Equal(t, []unstructured.Unstructured{
@@ -155,9 +155,9 @@ func Test_CertManagerProvider_AdditionalObjects(t *testing.T) {
155155
func Test_CertManagerProvider_GetCertSecretInfo(t *testing.T) {
156156
certProvier := certproviders.CertManagerCertificateProvider{}
157157
certInfo := certProvier.GetCertSecretInfo(render.CertificateProvisionerConfig{
158-
WebhookServiceName: "webhook-service",
159-
Namespace: "namespace",
160-
CertName: "cert-name",
158+
ServiceName: "webhook-service",
159+
Namespace: "namespace",
160+
CertName: "cert-name",
161161
})
162162
require.Equal(t, render.CertSecretInfo{
163163
SecretName: "cert-name",

‎internal/operator-controller/rukpak/render/certproviders/openshift_serviceca_test.go

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ func Test_OpenshiftServiceCAProvider_InjectCABundle(t *testing.T) {
2525
name: "injects inject-cabundle annotation in validating webhook configuration",
2626
obj: &admissionregistrationv1.ValidatingWebhookConfiguration{},
2727
cfg: render.CertificateProvisionerConfig{
28-
WebhookServiceName: "webhook-service",
29-
Namespace: "namespace",
30-
CertName: "cert-name",
28+
ServiceName: "webhook-service",
29+
Namespace: "namespace",
30+
CertName: "cert-name",
3131
},
3232
expectedObj: &admissionregistrationv1.ValidatingWebhookConfiguration{
3333
ObjectMeta: metav1.ObjectMeta{
@@ -41,9 +41,9 @@ func Test_OpenshiftServiceCAProvider_InjectCABundle(t *testing.T) {
4141
name: "injects inject-cabundle annotation in mutating webhook configuration",
4242
obj: &admissionregistrationv1.MutatingWebhookConfiguration{},
4343
cfg: render.CertificateProvisionerConfig{
44-
WebhookServiceName: "webhook-service",
45-
Namespace: "namespace",
46-
CertName: "cert-name",
44+
ServiceName: "webhook-service",
45+
Namespace: "namespace",
46+
CertName: "cert-name",
4747
},
4848
expectedObj: &admissionregistrationv1.MutatingWebhookConfiguration{
4949
ObjectMeta: metav1.ObjectMeta{
@@ -57,9 +57,9 @@ func Test_OpenshiftServiceCAProvider_InjectCABundle(t *testing.T) {
5757
name: "injects inject-cabundle annotation in custom resource definition",
5858
obj: &apiextensionsv1.CustomResourceDefinition{},
5959
cfg: render.CertificateProvisionerConfig{
60-
WebhookServiceName: "webhook-service",
61-
Namespace: "namespace",
62-
CertName: "cert-name",
60+
ServiceName: "webhook-service",
61+
Namespace: "namespace",
62+
CertName: "cert-name",
6363
},
6464
expectedObj: &apiextensionsv1.CustomResourceDefinition{
6565
ObjectMeta: metav1.ObjectMeta{
@@ -73,9 +73,9 @@ func Test_OpenshiftServiceCAProvider_InjectCABundle(t *testing.T) {
7373
name: "injects serving-cert-secret-name annotation in service resource referencing the certificate name",
7474
obj: &corev1.Service{},
7575
cfg: render.CertificateProvisionerConfig{
76-
WebhookServiceName: "webhook-service",
77-
Namespace: "namespace",
78-
CertName: "cert-name",
76+
ServiceName: "webhook-service",
77+
Namespace: "namespace",
78+
CertName: "cert-name",
7979
},
8080
expectedObj: &corev1.Service{
8181
ObjectMeta: metav1.ObjectMeta{
@@ -89,9 +89,9 @@ func Test_OpenshiftServiceCAProvider_InjectCABundle(t *testing.T) {
8989
name: "ignores other objects",
9090
obj: &corev1.Secret{},
9191
cfg: render.CertificateProvisionerConfig{
92-
WebhookServiceName: "webhook-service",
93-
Namespace: "namespace",
94-
CertName: "cert-name",
92+
ServiceName: "webhook-service",
93+
Namespace: "namespace",
94+
CertName: "cert-name",
9595
},
9696
expectedObj: &corev1.Secret{},
9797
},
@@ -107,9 +107,9 @@ func Test_OpenshiftServiceCAProvider_InjectCABundle(t *testing.T) {
107107
func Test_OpenshiftServiceCAProvider_AdditionalObjects(t *testing.T) {
108108
certProvider := certproviders.OpenshiftServiceCaCertificateProvider{}
109109
objs, err := certProvider.AdditionalObjects(render.CertificateProvisionerConfig{
110-
WebhookServiceName: "webhook-service",
111-
Namespace: "namespace",
112-
CertName: "cert-name",
110+
ServiceName: "webhook-service",
111+
Namespace: "namespace",
112+
CertName: "cert-name",
113113
})
114114
require.NoError(t, err)
115115
require.Nil(t, objs)
@@ -118,9 +118,9 @@ func Test_OpenshiftServiceCAProvider_AdditionalObjects(t *testing.T) {
118118
func Test_OpenshiftServiceCAProvider_GetCertSecretInfo(t *testing.T) {
119119
certProvider := certproviders.OpenshiftServiceCaCertificateProvider{}
120120
certInfo := certProvider.GetCertSecretInfo(render.CertificateProvisionerConfig{
121-
WebhookServiceName: "webhook-service",
122-
Namespace: "namespace",
123-
CertName: "cert-name",
121+
ServiceName: "webhook-service",
122+
Namespace: "namespace",
123+
CertName: "cert-name",
124124
})
125125
require.Equal(t, render.CertSecretInfo{
126126
SecretName: "cert-name",

‎internal/operator-controller/rukpak/render/registryv1/generators/generators.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ func BundleCRDGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.O
241241
ClientConfig: &apiextensionsv1.WebhookClientConfig{
242242
Service: &apiextensionsv1.ServiceReference{
243243
Namespace: opts.InstallNamespace,
244-
Name: certProvisioner.WebhookServiceName,
244+
Name: certProvisioner.ServiceName,
245245
Path: &conversionWebhookPath,
246246
Port: &cw.ContainerPort,
247247
},
@@ -314,7 +314,7 @@ func BundleValidatingWebhookResourceGenerator(rv1 *bundle.RegistryV1, opts rende
314314
ClientConfig: admissionregistrationv1.WebhookClientConfig{
315315
Service: &admissionregistrationv1.ServiceReference{
316316
Namespace: opts.InstallNamespace,
317-
Name: certProvisioner.WebhookServiceName,
317+
Name: certProvisioner.ServiceName,
318318
Path: wh.WebhookPath,
319319
Port: &wh.ContainerPort,
320320
},
@@ -362,7 +362,7 @@ func BundleMutatingWebhookResourceGenerator(rv1 *bundle.RegistryV1, opts render.
362362
ClientConfig: admissionregistrationv1.WebhookClientConfig{
363363
Service: &admissionregistrationv1.ServiceReference{
364364
Namespace: opts.InstallNamespace,
365-
Name: certProvisioner.WebhookServiceName,
365+
Name: certProvisioner.ServiceName,
366366
Path: wh.WebhookPath,
367367
Port: &wh.ContainerPort,
368368
},
@@ -379,10 +379,10 @@ func BundleMutatingWebhookResourceGenerator(rv1 *bundle.RegistryV1, opts render.
379379
return objs, nil
380380
}
381381

382-
// BundleWebhookServiceResourceGenerator generates Service resources based that support the webhooks defined in
383-
// the bundle's cluster service version spec. The resource is modified by the CertificateProvider in opts
382+
// BundleDeploymentServiceResourceGenerator generates Service resources that support, e.g. the webhooks,
383+
// defined in the bundle's cluster service version spec. The resource is modified by the CertificateProvider in opts
384384
// to add any annotations or modifications necessary for certificate injection.
385-
func BundleWebhookServiceResourceGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
385+
func BundleDeploymentServiceResourceGenerator(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
386386
if rv1 == nil {
387387
return nil, fmt.Errorf("bundle cannot be nil")
388388
}
@@ -415,7 +415,7 @@ func BundleWebhookServiceResourceGenerator(rv1 *bundle.RegistryV1, opts render.O
415415

416416
certProvisioner := render.CertProvisionerFor(deploymentSpec.Name, opts)
417417
serviceResource := CreateServiceResource(
418-
certProvisioner.WebhookServiceName,
418+
certProvisioner.ServiceName,
419419
opts.InstallNamespace,
420420
WithServiceSpec(
421421
corev1.ServiceSpec{

‎internal/operator-controller/rukpak/render/registryv1/generators/generators_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2000,7 +2000,7 @@ func Test_BundleMutatingWebhookResourceGenerator_FailsOnNil(t *testing.T) {
20002000
require.Contains(t, err.Error(), "bundle cannot be nil")
20012001
}
20022002

2003-
func Test_BundleWebhookServiceResourceGenerator_Succeeds(t *testing.T) {
2003+
func Test_BundleDeploymentServiceResourceGenerator_Succeeds(t *testing.T) {
20042004
fakeProvider := FakeCertProvider{
20052005
InjectCABundleFn: func(obj client.Object, cfg render.CertificateProvisionerConfig) error {
20062006
obj.SetAnnotations(map[string]string{
@@ -2414,14 +2414,14 @@ func Test_BundleWebhookServiceResourceGenerator_Succeeds(t *testing.T) {
24142414
},
24152415
} {
24162416
t.Run(tc.name, func(t *testing.T) {
2417-
objs, err := generators.BundleWebhookServiceResourceGenerator(tc.bundle, tc.opts)
2417+
objs, err := generators.BundleDeploymentServiceResourceGenerator(tc.bundle, tc.opts)
24182418
require.NoError(t, err)
24192419
require.Equal(t, tc.expectedResources, objs)
24202420
})
24212421
}
24222422
}
24232423

2424-
func Test_BundleWebhookServiceResourceGenerator_FailsOnNil(t *testing.T) {
2424+
func Test_BundleDeploymentServiceResourceGenerator_FailsOnNil(t *testing.T) {
24252425
objs, err := generators.BundleMutatingWebhookResourceGenerator(nil, render.Options{})
24262426
require.Nil(t, objs)
24272427
require.Error(t, err)

‎internal/operator-controller/rukpak/render/registryv1/registryv1.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,6 @@ var ResourceGenerators = []render.ResourceGenerator{
4343
generators.BundleCSVDeploymentGenerator,
4444
generators.BundleValidatingWebhookResourceGenerator,
4545
generators.BundleMutatingWebhookResourceGenerator,
46-
generators.BundleWebhookServiceResourceGenerator,
46+
generators.BundleDeploymentServiceResourceGenerator,
4747
generators.CertProviderResourceGenerator,
4848
}

‎internal/operator-controller/rukpak/render/registryv1/registryv1_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func Test_ResourceGeneratorsHasAllGenerators(t *testing.T) {
5050
generators.BundleCSVDeploymentGenerator,
5151
generators.BundleValidatingWebhookResourceGenerator,
5252
generators.BundleMutatingWebhookResourceGenerator,
53-
generators.BundleWebhookServiceResourceGenerator,
53+
generators.BundleDeploymentServiceResourceGenerator,
5454
generators.CertProviderResourceGenerator,
5555
}
5656
actualGenerators := registryv1.ResourceGenerators

0 commit comments

Comments
 (0)