Skip to content

Commit 8da8401

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #1933 from zcahana/fix_webhook_cabundle
Bug 1908596: Use correct caBundle for 'olmcahash' annotation
2 parents 072a93f + 2cf3849 commit 8da8401

File tree

2 files changed

+83
-51
lines changed

2 files changed

+83
-51
lines changed

‎pkg/controller/operators/olm/apiservices.go

Lines changed: 56 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -247,57 +247,62 @@ func (a *Operator) areAPIServicesAvailable(csv *v1alpha1.ClusterServiceVersion)
247247
return true, nil
248248
}
249249

250-
// getCABundle returns the CA associated with a deployment
251-
func (a *Operator) getCABundle(csv *v1alpha1.ClusterServiceVersion) ([]byte, error) {
252-
for _, desc := range csv.GetOwnedAPIServiceDescriptions() {
253-
apiServiceName := desc.GetName()
254-
apiService, err := a.lister.APIRegistrationV1().APIServiceLister().Get(apiServiceName)
250+
// getAPIServiceCABundle returns the CA associated with an API service
251+
func (a *Operator) getAPIServiceCABundle(csv *v1alpha1.ClusterServiceVersion, desc *v1alpha1.APIServiceDescription) ([]byte, error) {
252+
apiServiceName := desc.GetName()
253+
apiService, err := a.lister.APIRegistrationV1().APIServiceLister().Get(apiServiceName)
254+
255+
if err != nil {
256+
return nil, fmt.Errorf("could not retrieve generated APIService: %v", err)
257+
}
258+
259+
if len(apiService.Spec.CABundle) > 0 {
260+
return apiService.Spec.CABundle, nil
261+
}
262+
263+
return nil, fmt.Errorf("Unable to find ca")
264+
}
265+
266+
// getWebhookCABundle returns the CA associated with a webhook
267+
func (a *Operator) getWebhookCABundle(csv *v1alpha1.ClusterServiceVersion, desc *v1alpha1.WebhookDescription) ([]byte, error) {
268+
webhookLabels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)
269+
webhookLabels[install.WebhookDescKey] = desc.GenerateName
270+
webhookSelector := labels.SelectorFromSet(webhookLabels).String()
271+
272+
switch desc.Type {
273+
case v1alpha1.MutatingAdmissionWebhook:
274+
existingWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
255275
if err != nil {
256-
return nil, fmt.Errorf("could not retrieve generated APIService: %v", err)
257-
}
258-
if len(apiService.Spec.CABundle) > 0 {
259-
return apiService.Spec.CABundle, nil
276+
return nil, fmt.Errorf("could not retrieve generated MutatingWebhookConfiguration: %v", err)
260277
}
261-
}
262278

263-
for _, desc := range csv.Spec.WebhookDefinitions {
264-
webhookLabels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)
265-
webhookLabels[install.WebhookDescKey] = desc.GenerateName
266-
webhookSelector := labels.SelectorFromSet(webhookLabels).String()
279+
if len(existingWebhooks.Items) > 0 {
280+
return existingWebhooks.Items[0].Webhooks[0].ClientConfig.CABundle, nil
281+
}
282+
case v1alpha1.ValidatingAdmissionWebhook:
283+
existingWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
284+
if err != nil {
285+
return nil, fmt.Errorf("could not retrieve generated ValidatingWebhookConfiguration: %v", err)
286+
}
267287

268-
switch desc.Type {
269-
case v1alpha1.MutatingAdmissionWebhook:
270-
existingWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
288+
if len(existingWebhooks.Items) > 0 {
289+
return existingWebhooks.Items[0].Webhooks[0].ClientConfig.CABundle, nil
290+
}
291+
case v1alpha1.ConversionWebhook:
292+
for _, conversionCRD := range desc.ConversionCRDs {
293+
// check if CRD exists on cluster
294+
crd, err := a.opClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), conversionCRD, metav1.GetOptions{})
271295
if err != nil {
272-
return nil, fmt.Errorf("could not retrieve generated MutatingWebhookConfiguration: %v", err)
273-
}
274-
275-
if len(existingWebhooks.Items) > 0 {
276-
return existingWebhooks.Items[0].Webhooks[0].ClientConfig.CABundle, nil
296+
continue
277297
}
278-
case v1alpha1.ValidatingAdmissionWebhook:
279-
existingWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
280-
if err != nil {
281-
return nil, fmt.Errorf("could not retrieve generated ValidatingWebhookConfiguration: %v", err)
298+
if crd.Spec.Conversion == nil || crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil && crd.Spec.Conversion.Webhook.ClientConfig.CABundle == nil {
299+
continue
282300
}
283301

284-
if len(existingWebhooks.Items) > 0 {
285-
return existingWebhooks.Items[0].Webhooks[0].ClientConfig.CABundle, nil
286-
}
287-
case v1alpha1.ConversionWebhook:
288-
for _, conversionCRD := range desc.ConversionCRDs {
289-
// check if CRD exists on cluster
290-
crd, err := a.opClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), conversionCRD, metav1.GetOptions{})
291-
if err != nil {
292-
continue
293-
}
294-
if crd.Spec.Conversion == nil || crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil && crd.Spec.Conversion.Webhook.ClientConfig.CABundle == nil {
295-
continue
296-
}
297-
return crd.Spec.Conversion.Webhook.ClientConfig.CABundle, nil
298-
}
302+
return crd.Spec.Conversion.Webhook.ClientConfig.CABundle, nil
299303
}
300304
}
305+
301306
return nil, fmt.Errorf("Unable to find ca")
302307
}
303308

@@ -321,13 +326,13 @@ func (a *Operator) updateDeploymentSpecsWithApiServiceData(csv *v1alpha1.Cluster
321326
depSpecs[sddSpec.Name] = sddSpec.Spec
322327
}
323328

324-
caBundle, err := a.getCABundle(csv)
325-
if err != nil {
326-
return nil, fmt.Errorf("could not retrieve caBundle: %v", err)
327-
}
328-
caHash := certs.PEMSHA256(caBundle)
329-
330329
for _, desc := range csv.Spec.APIServiceDefinitions.Owned {
330+
caBundle, err := a.getAPIServiceCABundle(csv, &desc)
331+
if err != nil {
332+
return nil, fmt.Errorf("could not retrieve caBundle for owned APIServices %s: %v", fmt.Sprintf("%s.%s", desc.Version, desc.Group), err)
333+
}
334+
caHash := certs.PEMSHA256(caBundle)
335+
331336
depSpec, ok := depSpecs[desc.DeploymentName]
332337
if !ok {
333338
return nil, fmt.Errorf("StrategyDetailsDeployment missing deployment %s for owned APIServices %s", desc.DeploymentName, fmt.Sprintf("%s.%s", desc.Version, desc.Group))
@@ -349,6 +354,10 @@ func (a *Operator) updateDeploymentSpecsWithApiServiceData(csv *v1alpha1.Cluster
349354
}
350355

351356
for _, desc := range csv.Spec.WebhookDefinitions {
357+
caBundle, err := a.getWebhookCABundle(csv, &desc)
358+
if err != nil {
359+
return nil, fmt.Errorf("could not retrieve caBundle for WebhookDescription %s: %v", desc.GenerateName, err)
360+
}
352361
caHash := certs.PEMSHA256(caBundle)
353362

354363
depSpec, ok := depSpecs[desc.DeploymentName]

‎pkg/controller/operators/olm/operator_test.go

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3112,7 +3112,7 @@ func TestTransitionCSV(t *testing.T) {
31123112
}
31133113
}
31143114

3115-
func TestCaBundleRetrevial(t *testing.T) {
3115+
func TestWebhookCABundleRetrieval(t *testing.T) {
31163116
logrus.SetLevel(logrus.DebugLevel)
31173117
namespace := "ns"
31183118
missingCAError := fmt.Errorf("Unable to find ca")
@@ -3122,6 +3122,7 @@ func TestCaBundleRetrevial(t *testing.T) {
31223122
csvs []runtime.Object
31233123
crds []runtime.Object
31243124
objs []runtime.Object
3125+
desc v1alpha1.WebhookDescription
31253126
}
31263127
type expected struct {
31273128
caBundle []byte
@@ -3149,6 +3150,10 @@ func TestCaBundleRetrevial(t *testing.T) {
31493150
v1alpha1.CSVPhaseInstalling,
31503151
),
31513152
},
3153+
desc: v1alpha1.WebhookDescription{
3154+
GenerateName: "webhook",
3155+
Type: v1alpha1.ValidatingAdmissionWebhook,
3156+
},
31523157
},
31533158
expected: expected{
31543159
caBundle: nil,
@@ -3175,6 +3180,11 @@ func TestCaBundleRetrevial(t *testing.T) {
31753180
crds: []runtime.Object{
31763181
crdWithConversionWebhook(crd("c1", "v1", "g1"), caBundle),
31773182
},
3183+
desc: v1alpha1.WebhookDescription{
3184+
GenerateName: "webhook",
3185+
Type: v1alpha1.ConversionWebhook,
3186+
ConversionCRDs: []string{"c1.g1"},
3187+
},
31783188
},
31793189
expected: expected{
31803190
caBundle: caBundle,
@@ -3201,6 +3211,11 @@ func TestCaBundleRetrevial(t *testing.T) {
32013211
crds: []runtime.Object{
32023212
crd("c1", "v1", "g1"),
32033213
},
3214+
desc: v1alpha1.WebhookDescription{
3215+
GenerateName: "webhook",
3216+
Type: v1alpha1.ConversionWebhook,
3217+
ConversionCRDs: []string{"c1.g1"},
3218+
},
32043219
},
32053220
expected: expected{
32063221
caBundle: nil,
@@ -3233,7 +3248,7 @@ func TestCaBundleRetrevial(t *testing.T) {
32333248
"olm.owner": "csv1",
32343249
"olm.owner.namespace": namespace,
32353250
"olm.owner.kind": v1alpha1.ClusterServiceVersionKind,
3236-
"olm.webhook-description-generate-name": "",
3251+
"olm.webhook-description-generate-name": "webhook",
32373252
},
32383253
},
32393254
Webhooks: []admissionregistrationv1.ValidatingWebhook{
@@ -3246,6 +3261,10 @@ func TestCaBundleRetrevial(t *testing.T) {
32463261
},
32473262
},
32483263
},
3264+
desc: v1alpha1.WebhookDescription{
3265+
GenerateName: "webhook",
3266+
Type: v1alpha1.ValidatingAdmissionWebhook,
3267+
},
32493268
},
32503269
expected: expected{
32513270
caBundle: caBundle,
@@ -3278,7 +3297,7 @@ func TestCaBundleRetrevial(t *testing.T) {
32783297
"olm.owner": "csv1",
32793298
"olm.owner.namespace": namespace,
32803299
"olm.owner.kind": v1alpha1.ClusterServiceVersionKind,
3281-
"olm.webhook-description-generate-name": "",
3300+
"olm.webhook-description-generate-name": "webhook",
32823301
},
32833302
},
32843303
Webhooks: []admissionregistrationv1.MutatingWebhook{
@@ -3291,6 +3310,10 @@ func TestCaBundleRetrevial(t *testing.T) {
32913310
},
32923311
},
32933312
},
3313+
desc: v1alpha1.WebhookDescription{
3314+
GenerateName: "webhook",
3315+
Type: v1alpha1.MutatingAdmissionWebhook,
3316+
},
32943317
},
32953318
expected: expected{
32963319
caBundle: caBundle,
@@ -3315,7 +3338,7 @@ func TestCaBundleRetrevial(t *testing.T) {
33153338

33163339
// run csv sync for each CSV
33173340
for _, csv := range tt.initial.csvs {
3318-
caBundle, err := op.getCABundle(csv.(*v1alpha1.ClusterServiceVersion))
3341+
caBundle, err := op.getWebhookCABundle(csv.(*v1alpha1.ClusterServiceVersion), &tt.initial.desc)
33193342
require.Equal(t, tt.expected.err, err)
33203343
require.Equal(t, tt.expected.caBundle, caBundle)
33213344
}

0 commit comments

Comments
 (0)