Skip to content

Commit

Permalink
Merge pull request #5096 from sbueringer/pr-handle-v1beta1-crd-whc
Browse files Browse the repository at this point in the history
🐛 clusterctl: fix target namespace in v1beta1 CRDs and WebhookConfigurations
  • Loading branch information
k8s-ci-robot authored Aug 17, 2021
2 parents f23f263 + 68ef481 commit fae587a
Show file tree
Hide file tree
Showing 3 changed files with 265 additions and 16 deletions.
84 changes: 69 additions & 15 deletions cmd/clusterctl/client/repository/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import (

"github.com/pkg/errors"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
rbacv1 "k8s.io/api/rbac/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
Expand Down Expand Up @@ -318,7 +320,7 @@ func fixTargetNamespace(objs []unstructured.Unstructured, targetNamespace string

if o.GetKind() == mutatingWebhookConfigurationKind || o.GetKind() == validatingWebhookConfigurationKind || o.GetKind() == customResourceDefinitionKind {
var err error
o, err = fixWebHookNamespaceReferences(o, targetNamespace)
o, err = fixWebhookNamespaceReferences(o, targetNamespace)
if err != nil {
return nil, err
}
Expand All @@ -328,7 +330,7 @@ func fixTargetNamespace(objs []unstructured.Unstructured, targetNamespace string
return objs, nil
}

func fixWebHookNamespaceReferences(o unstructured.Unstructured, targetNamespace string) (unstructured.Unstructured, error) {
func fixWebhookNamespaceReferences(o unstructured.Unstructured, targetNamespace string) (unstructured.Unstructured, error) {
annotations := o.GetAnnotations()
secretNamespacedName, ok := annotations["cert-manager.io/inject-ca-from"]
if ok {
Expand All @@ -340,49 +342,101 @@ func fixWebHookNamespaceReferences(o unstructured.Unstructured, targetNamespace
o.SetAnnotations(annotations)
}

var err error
switch o.GetKind() {
case mutatingWebhookConfigurationKind:
return fixMutatingWebhookNamespaceReferences(o, targetNamespace)

case validatingWebhookConfigurationKind:
return fixValidatingWebhookNamespaceReferences(o, targetNamespace)

case customResourceDefinitionKind:
return fixCRDWebhookNamespaceReference(o, targetNamespace)
}

return o, errors.Errorf("failed to patch %s %s version", o.GroupVersionKind().Version, o.GetKind())
}

func fixMutatingWebhookNamespaceReferences(o unstructured.Unstructured, targetNamespace string) (unstructured.Unstructured, error) {
version := o.GroupVersionKind().Version
switch version {
case admissionregistrationv1beta1.SchemeGroupVersion.Version:
b := &admissionregistrationv1beta1.MutatingWebhookConfiguration{}
if err := scheme.Scheme.Convert(&o, b, nil); err != nil {
return o, err
}
for _, w := range b.Webhooks {
if w.ClientConfig.Service != nil {
w.ClientConfig.Service.Namespace = targetNamespace
}
}
return o, scheme.Scheme.Convert(b, &o, nil)
case admissionregistrationv1.SchemeGroupVersion.Version:
b := &admissionregistrationv1.MutatingWebhookConfiguration{}
if err := scheme.Scheme.Convert(&o, b, nil); err != nil {
return o, err
}

for _, w := range b.Webhooks {
if w.ClientConfig.Service != nil {
w.ClientConfig.Service.Namespace = targetNamespace
}
}
return o, scheme.Scheme.Convert(b, &o, nil)
}
return o, errors.Errorf("failed to patch %s MutatingWebhookConfiguration", version)
}

err = scheme.Scheme.Convert(b, &o, nil)

case validatingWebhookConfigurationKind:
func fixValidatingWebhookNamespaceReferences(o unstructured.Unstructured, targetNamespace string) (unstructured.Unstructured, error) {
version := o.GroupVersionKind().Version
switch version {
case admissionregistrationv1beta1.SchemeGroupVersion.Version:
b := &admissionregistrationv1beta1.ValidatingWebhookConfiguration{}
if err := scheme.Scheme.Convert(&o, b, nil); err != nil {
return o, err
}
for _, w := range b.Webhooks {
if w.ClientConfig.Service != nil {
w.ClientConfig.Service.Namespace = targetNamespace
}
}
return o, scheme.Scheme.Convert(b, &o, nil)
case admissionregistrationv1.SchemeGroupVersion.Version:
b := &admissionregistrationv1.ValidatingWebhookConfiguration{}
if err := scheme.Scheme.Convert(&o, b, nil); err != nil {
return o, err
}

for _, w := range b.Webhooks {
if w.ClientConfig.Service != nil {
w.ClientConfig.Service.Namespace = targetNamespace
}
}
return o, scheme.Scheme.Convert(b, &o, nil)
}
return o, errors.Errorf("failed to patch %s ValidatingWebhookConfiguration", version)
}
func fixCRDWebhookNamespaceReference(o unstructured.Unstructured, targetNamespace string) (unstructured.Unstructured, error) {
version := o.GroupVersionKind().Version
switch version {
case apiextensionsv1beta1.SchemeGroupVersion.Version:
crd := &apiextensionsv1beta1.CustomResourceDefinition{}
if err := scheme.Scheme.Convert(&o, crd, nil); err != nil {
return o, err
}
if crd.Spec.Conversion != nil && crd.Spec.Conversion.WebhookClientConfig != nil && crd.Spec.Conversion.WebhookClientConfig.Service != nil {
crd.Spec.Conversion.WebhookClientConfig.Service.Namespace = targetNamespace
}
return o, scheme.Scheme.Convert(crd, &o, nil)

err = scheme.Scheme.Convert(b, &o, nil)

case customResourceDefinitionKind:
case apiextensionsv1.SchemeGroupVersion.Version:
crd := &apiextensionsv1.CustomResourceDefinition{}
if err := scheme.Scheme.Convert(&o, crd, nil); err != nil {
return o, err
}

if crd.Spec.Conversion != nil && crd.Spec.Conversion.Webhook != nil && crd.Spec.Conversion.Webhook.ClientConfig != nil && crd.Spec.Conversion.Webhook.ClientConfig.Service != nil {
crd.Spec.Conversion.Webhook.ClientConfig.Service.Namespace = targetNamespace
}

err = scheme.Scheme.Convert(crd, &o, nil)
return o, scheme.Scheme.Convert(crd, &o, nil)
}
return o, err
return o, errors.Errorf("failed to patch %s CustomResourceDefinition", version)
}

// fixRBAC ensures all the ClusterRole and ClusterRoleBinding have the name prefixed with the namespace name and that
Expand Down
195 changes: 194 additions & 1 deletion cmd/clusterctl/client/repository/components_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ func Test_fixTargetNamespace(t *testing.T) {
objs []unstructured.Unstructured
targetNamespace string
want []unstructured.Unstructured
wantErr bool
}{
{
name: "fix Namespace object if exists",
Expand Down Expand Up @@ -203,6 +204,94 @@ func Test_fixTargetNamespace(t *testing.T) {
},
},
{
name: "fix v1beta1 webhook configs",
objs: []unstructured.Unstructured{
{
Object: map[string]interface{}{
"apiVersion": "admissionregistration.k8s.io/v1beta1",
"kind": "MutatingWebhookConfiguration",
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
"cert-manager.io/inject-ca-from": "capi-webhook-system/capm3-serving-cert",
},
"name": "capm3-mutating-webhook-configuration",
},
"webhooks": []interface{}{
map[string]interface{}{
"clientConfig": map[string]interface{}{
"caBundle": "Cg==",
"service": map[string]interface{}{
"name": "capm3-webhook-service",
"namespace": "capi-webhook-system",
"path": "/mutate-infrastructure-cluster-x-k8s-io-v1alpha4-metal3cluster",
},
},
},
},
},
},
},
targetNamespace: "bar",
want: []unstructured.Unstructured{
{
Object: map[string]interface{}{
"apiVersion": "admissionregistration.k8s.io/v1beta1",
"kind": "MutatingWebhookConfiguration",
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
"cert-manager.io/inject-ca-from": "bar/capm3-serving-cert",
},
"creationTimestamp": nil,
"name": "capm3-mutating-webhook-configuration",
},
"webhooks": []interface{}{
map[string]interface{}{
"name": "",
"clientConfig": map[string]interface{}{
"service": map[string]interface{}{
"name": "capm3-webhook-service",
"path": "/mutate-infrastructure-cluster-x-k8s-io-v1alpha4-metal3cluster",
"namespace": "bar",
},
"caBundle": "Cg==",
},
},
},
},
},
},
},
{
name: "unable to fix v1beta2 webhook configs",
objs: []unstructured.Unstructured{
{
Object: map[string]interface{}{
"apiVersion": "admissionregistration.k8s.io/v1beta2",
"kind": "MutatingWebhookConfiguration",
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
"cert-manager.io/inject-ca-from": "capi-webhook-system/capm3-serving-cert",
},
"name": "capm3-mutating-webhook-configuration",
},
"webhooks": []interface{}{
map[string]interface{}{
"clientConfig": map[string]interface{}{
"caBundle": "Cg==",
"service": map[string]interface{}{
"name": "capm3-webhook-service",
"namespace": "capi-webhook-system",
"path": "/mutate-infrastructure-cluster-x-k8s-io-v1alpha4-metal3cluster",
},
},
},
},
},
},
},
targetNamespace: "bar",
wantErr: true,
}, {
name: "fix v1 webhook configs",
objs: []unstructured.Unstructured{
{
Expand Down Expand Up @@ -263,7 +352,106 @@ func Test_fixTargetNamespace(t *testing.T) {
},
},
{
name: "fix crd webhook namespace",
name: "fix v1beta1 crd webhook namespace",
objs: []unstructured.Unstructured{
{
Object: map[string]interface{}{
"apiVersion": "apiextensions.k8s.io/v1beta1",
"kind": "CustomResourceDefinition",
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
"cert-manager.io/inject-ca-from": "capi-webhook-system/capm3-serving-cert",
},
"name": "aCoolName",
},
"spec": map[string]interface{}{
"conversion": map[string]interface{}{
"strategy": "Webhook",
"webhookClientConfig": map[string]interface{}{
"caBundle": "Cg==",
"service": map[string]interface{}{
"name": "capa-webhook-service",
"namespace": "capi-webhook-system",
"path": "/convert",
},
},
},
},
},
},
},
targetNamespace: "bar",
want: []unstructured.Unstructured{
{
Object: map[string]interface{}{
"apiVersion": "apiextensions.k8s.io/v1beta1",
"kind": "CustomResourceDefinition",
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
"cert-manager.io/inject-ca-from": "bar/capm3-serving-cert",
},
"creationTimestamp": nil,
"name": "aCoolName",
},
"spec": map[string]interface{}{
"group": "",
"names": map[string]interface{}{"plural": "", "kind": ""},
"scope": "",
"conversion": map[string]interface{}{
"strategy": "Webhook",
"webhookClientConfig": map[string]interface{}{
"caBundle": "Cg==",
"service": map[string]interface{}{
"name": "capa-webhook-service",
"namespace": "bar",
"path": "/convert",
},
},
},
},
"status": map[string]interface{}{
"storedVersions": nil,
"conditions": nil,
"acceptedNames": map[string]interface{}{"kind": "", "plural": ""},
},
},
},
},
},
{
name: "unable to fix v1beta2 crd webhook namespace",
objs: []unstructured.Unstructured{
{
Object: map[string]interface{}{
"apiVersion": "apiextensions.k8s.io/v1beta2",
"kind": "CustomResourceDefinition",
"metadata": map[string]interface{}{
"annotations": map[string]interface{}{
"cert-manager.io/inject-ca-from": "capi-webhook-system/capm3-serving-cert",
},
"name": "aCoolName",
},
"spec": map[string]interface{}{
"conversion": map[string]interface{}{
"strategy": "Webhook",
"webhookClientConfig": map[string]interface{}{
"caBundle": "Cg==",
"service": map[string]interface{}{
"name": "capa-webhook-service",
"namespace": "capi-webhook-system",
"path": "/convert",
},
},
},
},
},
},
},
targetNamespace: "bar",
wantErr: true,
},
{
name: "fix v1 crd webhook namespace",
objs: []unstructured.Unstructured{
{
Object: map[string]interface{}{
Expand Down Expand Up @@ -341,6 +529,11 @@ func Test_fixTargetNamespace(t *testing.T) {
g := NewWithT(t)

got, err := fixTargetNamespace(tt.objs, tt.targetNamespace)
if tt.wantErr {
g.Expect(err).To(HaveOccurred())
return
}

g.Expect(err).ToNot(HaveOccurred())
g.Expect(got).To(ContainElements(tt.want)) // skipping from test the automatically added namespace Object
})
Expand Down
2 changes: 2 additions & 0 deletions cmd/clusterctl/internal/scheme/scheme.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
admissionregistration "k8s.io/api/admissionregistration/v1"
admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
"k8s.io/apimachinery/pkg/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4"
Expand All @@ -38,6 +39,7 @@ func init() {
_ = clusterctlv1.AddToScheme(Scheme)
_ = clusterv1.AddToScheme(Scheme)
_ = apiextensionsv1.AddToScheme(Scheme)
_ = apiextensionsv1beta1.AddToScheme(Scheme)
_ = admissionregistration.AddToScheme(Scheme)
_ = admissionregistrationv1beta1.AddToScheme(Scheme)
_ = addonsv1.AddToScheme(Scheme)
Expand Down

0 comments on commit fae587a

Please sign in to comment.