Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 clusterctl: fix target namespace in v1beta1 CRDs and WebhookConfigurations #5096

Merged
merged 1 commit into from
Aug 17, 2021
Merged

🐛 clusterctl: fix target namespace in v1beta1 CRDs and WebhookConfigurations #5096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 69 additions & 15 deletions cmd/clusterctl/client/repository/components.go
View file
Open in desktop
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)
sbueringer marked this conversation as resolved.
Show resolved Hide resolved

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
View file
Open in desktop
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
View file
Open in desktop
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