Skip to content

Commit 055190e

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #1802 from openshift-cherrypick-robot/cherry-pick-1797-to-release-4.6
[release-4.6] Bug 1886448: Retrieve CA from conversion webhooks for CA Hash
2 parents 33ff880 + 23dc9da commit 055190e

File tree

2 files changed

+271
-5
lines changed

2 files changed

+271
-5
lines changed

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,9 @@ func (a *Operator) getCABundle(csv *v1alpha1.ClusterServiceVersion) ([]byte, err
265265
webhookLabels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)
266266
webhookLabels[install.WebhookDescKey] = desc.GenerateName
267267
webhookSelector := labels.SelectorFromSet(webhookLabels).String()
268-
if desc.Type == v1alpha1.MutatingAdmissionWebhook {
268+
269+
switch desc.Type {
270+
case v1alpha1.MutatingAdmissionWebhook:
269271
existingWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().MutatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
270272
if err != nil {
271273
return nil, fmt.Errorf("could not retrieve generated MutatingWebhookConfiguration: %v", err)
@@ -274,8 +276,7 @@ func (a *Operator) getCABundle(csv *v1alpha1.ClusterServiceVersion) ([]byte, err
274276
if len(existingWebhooks.Items) > 0 {
275277
return existingWebhooks.Items[0].Webhooks[0].ClientConfig.CABundle, nil
276278
}
277-
278-
} else if desc.Type == v1alpha1.ValidatingAdmissionWebhook {
279+
case v1alpha1.ValidatingAdmissionWebhook:
279280
existingWebhooks, err := a.opClient.KubernetesInterface().AdmissionregistrationV1().ValidatingWebhookConfigurations().List(context.TODO(), metav1.ListOptions{LabelSelector: webhookSelector})
280281
if err != nil {
281282
return nil, fmt.Errorf("could not retrieve generated ValidatingWebhookConfiguration: %v", err)
@@ -284,6 +285,18 @@ func (a *Operator) getCABundle(csv *v1alpha1.ClusterServiceVersion) ([]byte, err
284285
if len(existingWebhooks.Items) > 0 {
285286
return existingWebhooks.Items[0].Webhooks[0].ClientConfig.CABundle, nil
286287
}
288+
case v1alpha1.ConversionWebhook:
289+
for _, conversionCRD := range desc.ConversionCRDs {
290+
// check if CRD exists on cluster
291+
crd, err := a.opClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), conversionCRD, metav1.GetOptions{})
292+
if err != nil {
293+
continue
294+
}
295+
if crd.Spec.Conversion == nil || crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil && crd.Spec.Conversion.Webhook.ClientConfig.CABundle == nil {
296+
continue
297+
}
298+
return crd.Spec.Conversion.Webhook.ClientConfig.CABundle, nil
299+
}
287300
}
288301
}
289302
return nil, fmt.Errorf("Unable to find ca")
@@ -480,7 +493,6 @@ func (a *Operator) updateDeploymentSpecsWithApiServiceData(csv *v1alpha1.Cluster
480493

481494
func (a *Operator) cleanUpRemovedWebhooks(csv *v1alpha1.ClusterServiceVersion) error {
482495
webhookLabels := ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)
483-
webhookLabels = ownerutil.OwnerLabel(csv, v1alpha1.ClusterServiceVersionKind)
484496
webhookSelector := labels.SelectorFromSet(webhookLabels).String()
485497

486498
csvWebhookGenerateNames := make(map[string]struct{}, len(csv.Spec.WebhookDefinitions))
@@ -562,7 +574,7 @@ func (a *Operator) areWebhooksAvailable(csv *v1alpha1.ClusterServiceVersion) (bo
562574
return false, err
563575
}
564576

565-
if crd.Spec.Conversion.Strategy != "Webhook" || crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil && crd.Spec.Conversion.Webhook.ClientConfig.CABundle == nil {
577+
if crd.Spec.Conversion == nil || crd.Spec.Conversion.Strategy != "Webhook" || crd.Spec.Conversion.Webhook == nil || crd.Spec.Conversion.Webhook.ClientConfig == nil && crd.Spec.Conversion.Webhook.ClientConfig.CABundle == nil {
566578
return false, fmt.Errorf("ConversionWebhook not ready")
567579
}
568580
webhookCount++

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

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/sirupsen/logrus"
2222
"github.com/stretchr/testify/assert"
2323
"github.com/stretchr/testify/require"
24+
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
2425
appsv1 "k8s.io/api/apps/v1"
2526
corev1 "k8s.io/api/core/v1"
2627
rbacv1 "k8s.io/api/rbac/v1"
@@ -3091,6 +3092,217 @@ func TestTransitionCSV(t *testing.T) {
30913092
}
30923093
}
30933094

3095+
func TestCaBundleRetrevial(t *testing.T) {
3096+
logrus.SetLevel(logrus.DebugLevel)
3097+
namespace := "ns"
3098+
missingCAError := fmt.Errorf("Unable to find ca")
3099+
caBundle := []byte("Foo")
3100+
3101+
type initial struct {
3102+
csvs []runtime.Object
3103+
crds []runtime.Object
3104+
objs []runtime.Object
3105+
}
3106+
type expected struct {
3107+
caBundle []byte
3108+
err error
3109+
}
3110+
tests := []struct {
3111+
name string
3112+
initial initial
3113+
expected expected
3114+
}{
3115+
{
3116+
name: "MissingCAResource",
3117+
initial: initial{
3118+
csvs: []runtime.Object{
3119+
csv("csv1",
3120+
namespace,
3121+
"0.0.0",
3122+
"",
3123+
installStrategy("csv1-dep1",
3124+
nil,
3125+
[]v1alpha1.StrategyDeploymentPermissions{},
3126+
),
3127+
[]*apiextensionsv1.CustomResourceDefinition{crd("c1", "v1", "g1")},
3128+
[]*apiextensionsv1.CustomResourceDefinition{},
3129+
v1alpha1.CSVPhaseInstalling,
3130+
),
3131+
},
3132+
},
3133+
expected: expected{
3134+
caBundle: nil,
3135+
err: missingCAError,
3136+
},
3137+
},
3138+
{
3139+
name: "RetrieveCAFromConversionWebhook",
3140+
initial: initial{
3141+
csvs: []runtime.Object{
3142+
csvWithConversionWebhook(csv("csv1",
3143+
namespace,
3144+
"0.0.0",
3145+
"",
3146+
installStrategy("csv1-dep1",
3147+
nil,
3148+
[]v1alpha1.StrategyDeploymentPermissions{},
3149+
),
3150+
[]*apiextensionsv1.CustomResourceDefinition{crd("c1", "v1", "g1")},
3151+
[]*apiextensionsv1.CustomResourceDefinition{},
3152+
v1alpha1.CSVPhaseInstalling,
3153+
), "csv1-dep1", []string{"c1.g1"}),
3154+
},
3155+
crds: []runtime.Object{
3156+
crdWithConversionWebhook(crd("c1", "v1", "g1"), caBundle),
3157+
},
3158+
},
3159+
expected: expected{
3160+
caBundle: caBundle,
3161+
err: nil,
3162+
},
3163+
},
3164+
{
3165+
name: "FailToRetrieveCAFromConversionWebhook",
3166+
initial: initial{
3167+
csvs: []runtime.Object{
3168+
csvWithConversionWebhook(csv("csv1",
3169+
namespace,
3170+
"0.0.0",
3171+
"",
3172+
installStrategy("csv1-dep1",
3173+
nil,
3174+
[]v1alpha1.StrategyDeploymentPermissions{},
3175+
),
3176+
[]*apiextensionsv1.CustomResourceDefinition{crd("c1", "v1", "g1")},
3177+
[]*apiextensionsv1.CustomResourceDefinition{},
3178+
v1alpha1.CSVPhaseInstalling,
3179+
), "csv1-dep1", []string{"c1.g1"}),
3180+
},
3181+
crds: []runtime.Object{
3182+
crd("c1", "v1", "g1"),
3183+
},
3184+
},
3185+
expected: expected{
3186+
caBundle: nil,
3187+
err: missingCAError,
3188+
},
3189+
},
3190+
{
3191+
name: "RetrieveFromValidatingAdmissionWebhook",
3192+
initial: initial{
3193+
csvs: []runtime.Object{
3194+
csvWithValidatingAdmissionWebhook(csv("csv1",
3195+
namespace,
3196+
"0.0.0",
3197+
"",
3198+
installStrategy("csv1-dep1",
3199+
nil,
3200+
[]v1alpha1.StrategyDeploymentPermissions{},
3201+
),
3202+
[]*apiextensionsv1.CustomResourceDefinition{crd("c1", "v1", "g1")},
3203+
[]*apiextensionsv1.CustomResourceDefinition{},
3204+
v1alpha1.CSVPhaseInstalling,
3205+
), "csv1-dep1", []string{"c1.g1"}),
3206+
},
3207+
objs: []runtime.Object{
3208+
&admissionregistrationv1.ValidatingWebhookConfiguration{
3209+
ObjectMeta: metav1.ObjectMeta{
3210+
Name: "webhook",
3211+
Namespace: namespace,
3212+
Labels: map[string]string{
3213+
"olm.owner": "csv1",
3214+
"olm.owner.namespace": namespace,
3215+
"olm.owner.kind": v1alpha1.ClusterServiceVersionKind,
3216+
"olm.webhook-description-generate-name": "",
3217+
},
3218+
},
3219+
Webhooks: []admissionregistrationv1.ValidatingWebhook{
3220+
{
3221+
Name: "Webhook",
3222+
ClientConfig: admissionregistrationv1.WebhookClientConfig{
3223+
CABundle: caBundle,
3224+
},
3225+
},
3226+
},
3227+
},
3228+
},
3229+
},
3230+
expected: expected{
3231+
caBundle: caBundle,
3232+
err: nil,
3233+
},
3234+
},
3235+
{
3236+
name: "RetrieveFromMutatingAdmissionWebhook",
3237+
initial: initial{
3238+
csvs: []runtime.Object{
3239+
csvWithMutatingAdmissionWebhook(csv("csv1",
3240+
namespace,
3241+
"0.0.0",
3242+
"",
3243+
installStrategy("csv1-dep1",
3244+
nil,
3245+
[]v1alpha1.StrategyDeploymentPermissions{},
3246+
),
3247+
[]*apiextensionsv1.CustomResourceDefinition{crd("c1", "v1", "g1")},
3248+
[]*apiextensionsv1.CustomResourceDefinition{},
3249+
v1alpha1.CSVPhaseInstalling,
3250+
), "csv1-dep1", []string{"c1.g1"}),
3251+
},
3252+
objs: []runtime.Object{
3253+
&admissionregistrationv1.MutatingWebhookConfiguration{
3254+
ObjectMeta: metav1.ObjectMeta{
3255+
Name: "webhook",
3256+
Namespace: namespace,
3257+
Labels: map[string]string{
3258+
"olm.owner": "csv1",
3259+
"olm.owner.namespace": namespace,
3260+
"olm.owner.kind": v1alpha1.ClusterServiceVersionKind,
3261+
"olm.webhook-description-generate-name": "",
3262+
},
3263+
},
3264+
Webhooks: []admissionregistrationv1.MutatingWebhook{
3265+
{
3266+
Name: "Webhook",
3267+
ClientConfig: admissionregistrationv1.WebhookClientConfig{
3268+
CABundle: caBundle,
3269+
},
3270+
},
3271+
},
3272+
},
3273+
},
3274+
},
3275+
expected: expected{
3276+
caBundle: caBundle,
3277+
err: nil,
3278+
},
3279+
},
3280+
}
3281+
for _, tt := range tests {
3282+
t.Run(tt.name, func(t *testing.T) {
3283+
// Create test operator
3284+
ctx, cancel := context.WithCancel(context.TODO())
3285+
defer cancel()
3286+
op, err := NewFakeOperator(
3287+
ctx,
3288+
withNamespaces(namespace, "kube-system"),
3289+
withClientObjs(tt.initial.csvs...),
3290+
withK8sObjs(tt.initial.objs...),
3291+
withExtObjs(tt.initial.crds...),
3292+
withOperatorNamespace(namespace),
3293+
)
3294+
require.NoError(t, err)
3295+
3296+
// run csv sync for each CSV
3297+
for _, csv := range tt.initial.csvs {
3298+
caBundle, err := op.getCABundle(csv.(*v1alpha1.ClusterServiceVersion))
3299+
require.Equal(t, tt.expected.err, err)
3300+
require.Equal(t, tt.expected.caBundle, caBundle)
3301+
}
3302+
})
3303+
}
3304+
}
3305+
30943306
// TestUpdates verifies that a set of expected phase transitions occur when multiple CSVs are present
30953307
// and that they do not depend on sync order or event order
30963308
func TestUpdates(t *testing.T) {
@@ -4531,3 +4743,45 @@ func TestAPIServiceResourceErrorActionable(t *testing.T) {
45314743
}
45324744

45334745
}
4746+
4747+
func crdWithConversionWebhook(crd *apiextensionsv1.CustomResourceDefinition, caBundle []byte) *apiextensionsv1.CustomResourceDefinition {
4748+
crd.Spec.Conversion = &apiextensionsv1.CustomResourceConversion{
4749+
Strategy: "Webhook",
4750+
Webhook: &apiextensionsv1.WebhookConversion{
4751+
ConversionReviewVersions: []string{"v1beta1"},
4752+
ClientConfig: &apiextensionsv1.WebhookClientConfig{
4753+
CABundle: caBundle,
4754+
},
4755+
},
4756+
}
4757+
return crd
4758+
}
4759+
4760+
func csvWithConversionWebhook(csv *v1alpha1.ClusterServiceVersion, deploymentName string, conversionCRDs []string) *v1alpha1.ClusterServiceVersion {
4761+
return csvWithWebhook(csv, deploymentName, conversionCRDs, v1alpha1.ConversionWebhook)
4762+
}
4763+
4764+
func csvWithValidatingAdmissionWebhook(csv *v1alpha1.ClusterServiceVersion, deploymentName string, conversionCRDs []string) *v1alpha1.ClusterServiceVersion {
4765+
return csvWithWebhook(csv, deploymentName, conversionCRDs, v1alpha1.ValidatingAdmissionWebhook)
4766+
}
4767+
4768+
func csvWithMutatingAdmissionWebhook(csv *v1alpha1.ClusterServiceVersion, deploymentName string, conversionCRDs []string) *v1alpha1.ClusterServiceVersion {
4769+
return csvWithWebhook(csv, deploymentName, conversionCRDs, v1alpha1.MutatingAdmissionWebhook)
4770+
}
4771+
4772+
func csvWithWebhook(csv *v1alpha1.ClusterServiceVersion, deploymentName string, conversionCRDs []string, webhookType v1alpha1.WebhookAdmissionType) *v1alpha1.ClusterServiceVersion {
4773+
sideEffectNone := admissionregistrationv1.SideEffectClassNone
4774+
targetPort := intstr.FromInt(443)
4775+
csv.Spec.WebhookDefinitions = []v1alpha1.WebhookDescription{
4776+
{
4777+
Type: webhookType,
4778+
DeploymentName: deploymentName,
4779+
ContainerPort: 443,
4780+
TargetPort: &targetPort,
4781+
SideEffects: &sideEffectNone,
4782+
ConversionCRDs: conversionCRDs,
4783+
AdmissionReviewVersions: []string{"v1beta1"},
4784+
},
4785+
}
4786+
return csv
4787+
}

0 commit comments

Comments
 (0)