From 25b69006273f12ade46de9e6155f080be0901a08 Mon Sep 17 00:00:00 2001 From: shubham Date: Fri, 24 Jan 2020 13:17:31 +0530 Subject: [PATCH 1/9] fix(cspc, webhook): add validation for cspc deletion Signed-off-by: shubham --- pkg/webhook/configuration.go | 5 +++-- pkg/webhook/cspc.go | 38 +++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/pkg/webhook/configuration.go b/pkg/webhook/configuration.go index 1507f8bac6..60cf3973e8 100644 --- a/pkg/webhook/configuration.go +++ b/pkg/webhook/configuration.go @@ -163,6 +163,7 @@ func createAdmissionService( Operations: []v1beta1.OperationType{ v1beta1.Create, v1beta1.Update, + v1beta1.Delete, }, Rule: v1beta1.Rule{ APIGroups: []string{"*"}, @@ -447,7 +448,7 @@ func preUpgrade(openebsNamespace string) error { } for _, service := range svcList.Items { - if len(service.Labels["openebs.io/version"]) == 0 { + if service.Labels["openebs.io/version"] != version.Current() { err = svc.NewKubeClient(svc.WithNamespace(openebsNamespace)).Delete(service.Name, &metav1.DeleteOptions{}) if err != nil { return fmt.Errorf("failed to delete old service %s: %s", service.Name, err.Error()) @@ -460,7 +461,7 @@ func preUpgrade(openebsNamespace string) error { } for _, config := range webhookConfigList.Items { - if len(config.Labels["openebs.io/version"]) == 0 { + if config.Labels["openebs.io/version"] != version.Current() { err = validate.KubeClient().Delete(config.Name, &metav1.DeleteOptions{}) if err != nil { return fmt.Errorf("failed to delete older webhook config %s: %s", config.Name, err.Error()) diff --git a/pkg/webhook/cspc.go b/pkg/webhook/cspc.go index 5b10a513b6..2e4d149127 100644 --- a/pkg/webhook/cspc.go +++ b/pkg/webhook/cspc.go @@ -28,6 +28,8 @@ import ( blockdevice "github.com/openebs/maya/pkg/blockdevice/v1alpha2" blockdeviceclaim "github.com/openebs/maya/pkg/blockdeviceclaim/v1alpha1" cspcv1alpha1 "github.com/openebs/maya/pkg/cstor/poolcluster/v1alpha1" + cspi "github.com/openebs/maya/pkg/cstor/poolinstance/v1alpha3" + cvr "github.com/openebs/maya/pkg/cstor/volumereplica/v1alpha1" env "github.com/openebs/maya/pkg/env/v1alpha1" "github.com/pkg/errors" "k8s.io/api/admission/v1beta1" @@ -103,9 +105,12 @@ func (wh *webhook) validateCSPC(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionR } else if req.Operation == v1beta1.Create { klog.V(5).Infof("Admission webhook create request for type %s", req.Kind.Kind) return wh.validateCSPCCreateRequest(req) + } else if req.Operation == v1beta1.Delete { + klog.V(5).Infof("Admission webhook delete request for type %s", req.Kind.Kind) + return wh.validateCSPCDeleteRequest(req) } - klog.V(2).Info("Admission wehbook for PVC not " + + klog.V(2).Info("Admission wehbook for CSPC not " + "configured for operations other than UPDATE and CREATE") return response } @@ -128,6 +133,37 @@ func (wh *webhook) validateCSPCCreateRequest(req *v1beta1.AdmissionRequest) *v1b return response } +// validateCSPCDeleteRequest validates CSPC delete request +// if any cvrs exist on the cspc pools then deletion is invalid +func (wh *webhook) validateCSPCDeleteRequest(req *v1beta1.AdmissionRequest) *v1beta1.AdmissionResponse { + response := NewAdmissionResponse().SetAllowed().WithResultAsSuccess(http.StatusAccepted).AR + cspiList, err := cspi.NewKubeClient().WithNamespace(req.Namespace).List( + metav1.ListOptions{ + LabelSelector: string(apis.CStorPoolClusterCPK) + "=" + req.Name, + }) + if err != nil { + klog.Errorf("Could not list cspi for cspc %s: %s", req.Name, err.Error()) + response = BuildForAPIObject(response).UnSetAllowed().WithResultAsFailure(err, http.StatusBadRequest).AR + return response + } + for _, cspiObj := range cspiList.Items { + cvrList, err := cvr.NewKubeclient().WithNamespace(cspiObj.Namespace).List(metav1.ListOptions{ + LabelSelector: "cstorpoolinstance.openebs.io/name=" + cspiObj.Name, + }) + if err != nil { + klog.Errorf("Could not list cvr for cspi %s: %s", cspiObj.Name, err.Error()) + response = BuildForAPIObject(response).UnSetAllowed().WithResultAsFailure(err, http.StatusBadRequest).AR + return response + } + if len(cvrList.Items) != 0 { + err := errors.Errorf("invalid cspc %s deletion: volume still exists on pool %s", req.Name, cspiObj.Name) + response = BuildForAPIObject(response).UnSetAllowed().WithResultAsFailure(err, http.StatusUnprocessableEntity).AR + return response + } + } + return response +} + func cspcValidation(cspc *apis.CStorPoolCluster) (bool, string) { usedNodes := map[string]bool{} if len(cspc.Spec.Pools) == 0 { From 8bcc566e7c7e20eeb40aac84f9b76e59cf625830 Mon Sep 17 00:00:00 2001 From: shubham Date: Fri, 24 Jan 2020 19:07:34 +0530 Subject: [PATCH 2/9] remove not required logs Signed-off-by: shubham --- pkg/webhook/cspc.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/webhook/cspc.go b/pkg/webhook/cspc.go index 2e4d149127..e66490673e 100644 --- a/pkg/webhook/cspc.go +++ b/pkg/webhook/cspc.go @@ -110,8 +110,6 @@ func (wh *webhook) validateCSPC(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionR return wh.validateCSPCDeleteRequest(req) } - klog.V(2).Info("Admission wehbook for CSPC not " + - "configured for operations other than UPDATE and CREATE") return response } From 514f57fa2a9e6368144678f44eefddc7800178ef Mon Sep 17 00:00:00 2001 From: shubham Date: Mon, 27 Jan 2020 15:37:42 +0530 Subject: [PATCH 3/9] fix pre-upgrade for webhook Signed-off-by: shubham --- pkg/kubernetes/secret/kubernetes.go | 23 ++++- pkg/kubernetes/service/v1alpha1/kubernetes.go | 40 +++++++++ .../webhook/validate/v1alpha1/kubernetes.go | 29 ++++++- pkg/webhook/configuration.go | 83 ++++++++++++++++--- 4 files changed, 162 insertions(+), 13 deletions(-) diff --git a/pkg/kubernetes/secret/kubernetes.go b/pkg/kubernetes/secret/kubernetes.go index 9dfa6695bd..18aef3ed11 100644 --- a/pkg/kubernetes/secret/kubernetes.go +++ b/pkg/kubernetes/secret/kubernetes.go @@ -40,6 +40,10 @@ type getFn func(cli *kubernetes.Clientset, namespace, name string, opts metav1.G // to create secret type createFn func(cli *kubernetes.Clientset, namespace string, secret *corev1.Secret) (*corev1.Secret, error) +// updateFn is a typed function that abstracts +// to update secret +type updateFn func(cli *kubernetes.Clientset, namespace string, secret *corev1.Secret) (*corev1.Secret, error) + // listFn is a typed function that abstracts listing of secret instances type listFn func(cli *kubernetes.Clientset, namespace string, opts metav1.ListOptions) (*corev1.SecretList, error) @@ -68,6 +72,7 @@ type Kubeclient struct { create createFn del deleteFn list listFn + update updateFn } // KubeClientBuildOption defines the abstraction @@ -100,7 +105,11 @@ func (k *Kubeclient) withDefaults() { return cli.CoreV1().Secrets(namespace).List(opts) } } - + if k.update == nil { + k.update = func(cli *kubernetes.Clientset, namespace string, secret *corev1.Secret) (*corev1.Secret, error) { + return cli.CoreV1().Secrets(namespace).Update(secret) + } + } if k.del == nil { k.del = func(cli *kubernetes.Clientset, namespace, name string, opts *metav1.DeleteOptions) error { return cli.CoreV1().Secrets(namespace).Delete(name, opts) @@ -202,3 +211,15 @@ func (k *Kubeclient) List(opts metav1.ListOptions) (*corev1.SecretList, error) { } return k.list(cli, k.namespace, opts) } + +// Update updates and returns updated secret instance +func (k *Kubeclient) Update(secret *corev1.Secret) (*corev1.Secret, error) { + if secret == nil { + return nil, errors.New("failed to update secret: nil secret object") + } + cli, err := k.getClientsetOrCached() + if err != nil { + return nil, errors.Wrapf(err, "failed to update secret") + } + return k.update(cli, k.namespace, secret) +} diff --git a/pkg/kubernetes/service/v1alpha1/kubernetes.go b/pkg/kubernetes/service/v1alpha1/kubernetes.go index 5d642f2812..01b3a29c9b 100644 --- a/pkg/kubernetes/service/v1alpha1/kubernetes.go +++ b/pkg/kubernetes/service/v1alpha1/kubernetes.go @@ -65,6 +65,13 @@ type createFn func( namespace string, ) (*corev1.Service, error) +// updateFn is a typed function that abstracts delete of service instances +type updateFn func( + cli *kubernetes.Clientset, + service *corev1.Service, + namespace string, +) (*corev1.Service, error) + // patchFn is a typed function that abstracts patch of service instances type patchFn func( cli *kubernetes.Clientset, @@ -92,6 +99,7 @@ type Kubeclient struct { del delFn create createFn patch patchFn + update updateFn } // KubeclientBuildOption defines the abstraction to build a kubeclient instance @@ -170,6 +178,18 @@ func defaultCreate( Create(service) } +// defaultUpdate is the default implementation to update +// a service instance in kubernetes cluster +func defaultUpdate( + cli *kubernetes.Clientset, + service *corev1.Service, + namespace string, +) (*corev1.Service, error) { + return cli.CoreV1(). + Services(namespace). + Update(service) +} + // defaultPatch is the default implementation to patch // a service instance in kubernetes cluster func defaultPatch( @@ -207,6 +227,9 @@ func (k *Kubeclient) withDefaults() { if k.patch == nil { k.patch = defaultPatch } + if k.update == nil { + k.update = defaultUpdate + } } // WithClientset sets the kubernetes client against the kubeclient instance @@ -336,6 +359,23 @@ func (k *Kubeclient) Create(service *corev1.Service) (*corev1.Service, error) { return k.create(cli, service, k.namespace) } +// Update updates a service in specified namespace in kubernetes cluster +func (k *Kubeclient) Update(service *corev1.Service) (*corev1.Service, error) { + if service == nil { + return nil, errors.New("failed to create service: nil service object") + } + cli, err := k.getClientOrCached() + if err != nil { + return nil, errors.Wrapf( + err, + "failed to create service {%s} in namespace {%s}", + service.Name, + service.Namespace, + ) + } + return k.update(cli, service, k.namespace) +} + // Patch patches service object for given name func (k *Kubeclient) Patch( name string, diff --git a/pkg/kubernetes/webhook/validate/v1alpha1/kubernetes.go b/pkg/kubernetes/webhook/validate/v1alpha1/kubernetes.go index ec490df96c..8d5a132fcb 100644 --- a/pkg/kubernetes/webhook/validate/v1alpha1/kubernetes.go +++ b/pkg/kubernetes/webhook/validate/v1alpha1/kubernetes.go @@ -52,6 +52,14 @@ type createFunc func(cli *kubernetes.Clientset, error, ) +// updateFn is a typed function that abstracts +// to update admissionwebhook configuration +type updateFn func(cli *kubernetes.Clientset, + config *admission.ValidatingWebhookConfiguration) ( + *admission.ValidatingWebhookConfiguration, + error, +) + // Kubeclient enables kubernetes API operations // on upgrade result instance type Kubeclient struct { @@ -66,6 +74,7 @@ type Kubeclient struct { create createFunc get getFunc del delFunc + update updateFn } // KubeclientBuildOption defines the abstraction @@ -103,9 +112,12 @@ func (k *Kubeclient) withDefaults() { k.del = func(cs *kubernetes.Clientset, name string, opts *metav1.DeleteOptions) error { return cs.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Delete(name, opts) } - } - + if k.update == nil { + k.update = func(cs *kubernetes.Clientset, config *admission.ValidatingWebhookConfiguration) (*admission.ValidatingWebhookConfiguration, error) { + return cs.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Update(config) + } + } } // WithClientset sets the kubernetes clientset against @@ -197,3 +209,16 @@ func (k *Kubeclient) Delete(name string, options *metav1.DeleteOptions) error { } return k.del(cli, name, options) } + +// Update updates validatingWebhookConfiguration, and returns the updated +// corresponding validatingWebhookConfiguration object, and an error if there is any. +func (k *Kubeclient) Update(config *admission.ValidatingWebhookConfiguration) (*admission.ValidatingWebhookConfiguration, error) { + if config == nil { + return nil, errors.New("failed to create validating configuration: nil configuration") + } + cs, err := k.getClientOrCached() + if err != nil { + return nil, err + } + return k.update(cs, config) +} diff --git a/pkg/webhook/configuration.go b/pkg/webhook/configuration.go index 60cf3973e8..11e7e3038b 100644 --- a/pkg/webhook/configuration.go +++ b/pkg/webhook/configuration.go @@ -26,6 +26,7 @@ import ( secret "github.com/openebs/maya/pkg/kubernetes/secret" svc "github.com/openebs/maya/pkg/kubernetes/service/v1alpha1" validate "github.com/openebs/maya/pkg/kubernetes/webhook/validate/v1alpha1" + "github.com/openebs/maya/pkg/util" "github.com/openebs/maya/pkg/version" "github.com/pkg/errors" "k8s.io/api/admissionregistration/v1beta1" @@ -424,6 +425,32 @@ func GetAdmissionReference() (*metav1.OwnerReference, error) { return nil, fmt.Errorf("failed to create deployment ownerReference") } +type transformSvcFunc func(*corev1.Service) +type transformSecretFunc func(*corev1.Secret) +type transformConfigFunc func(*v1beta1.ValidatingWebhookConfiguration) + +func addCSPCDeleteRule(config *v1beta1.ValidatingWebhookConfiguration) { + if config.Labels["openebs.io/version"] < "1.7.0" { + index := -1 + for i, rule := range config.Webhooks[0].Rules { + if util.ContainsString(rule.Rule.Resources, "cstorpoolclusters") { + index = i + } + } + if index != -1 { + config.Webhooks[0].Rules[index].Operations = append(config.Webhooks[0].Rules[index].Operations, v1beta1.Delete) + } + } +} + +var ( + transformSecret = []transformSecretFunc{} + transformSvc = []transformSvcFunc{} + transformConfig = []transformConfigFunc{ + addCSPCDeleteRule, + } +) + // preUpgrade checks for the required older webhook configs,older // then 1.4.0 if exists delete them. func preUpgrade(openebsNamespace string) error { @@ -434,10 +461,22 @@ func preUpgrade(openebsNamespace string) error { } for _, scrt := range secretlist.Items { - if len(scrt.Labels["openebs.io/version"]) == 0 { - err = secret.NewKubeClient(secret.WithNamespace(openebsNamespace)).Delete(scrt.Name, &metav1.DeleteOptions{}) - if err != nil { - return fmt.Errorf("failed to delete old secret %s: %s", scrt.Name, err.Error()) + if scrt.Labels["openebs.io/version"] != version.Current() { + if scrt.Labels["openebs.io/version"] == "" { + err = secret.NewKubeClient(secret.WithNamespace(openebsNamespace)).Delete(scrt.Name, &metav1.DeleteOptions{}) + if err != nil { + return fmt.Errorf("failed to delete old secret %s: %s", scrt.Name, err.Error()) + } + } else { + newScrt := &scrt + for _, t := range transformSecret { + t(newScrt) + } + newScrt.Labels["openebs.io/version"] = version.Current() + _, err = secret.NewKubeClient(secret.WithNamespace(openebsNamespace)).Update(newScrt) + if err != nil { + return fmt.Errorf("failed to update old secret %s: %s", scrt.Name, err.Error()) + } } } } @@ -449,9 +488,21 @@ func preUpgrade(openebsNamespace string) error { for _, service := range svcList.Items { if service.Labels["openebs.io/version"] != version.Current() { - err = svc.NewKubeClient(svc.WithNamespace(openebsNamespace)).Delete(service.Name, &metav1.DeleteOptions{}) - if err != nil { - return fmt.Errorf("failed to delete old service %s: %s", service.Name, err.Error()) + if service.Labels["openebs.io/version"] == "" { + err = svc.NewKubeClient(svc.WithNamespace(openebsNamespace)).Delete(service.Name, &metav1.DeleteOptions{}) + if err != nil { + return fmt.Errorf("failed to delete old service %s: %s", service.Name, err.Error()) + } + } else { + newSvc := &service + for _, t := range transformSvc { + t(newSvc) + } + newSvc.Labels["openebs.io/version"] = version.Current() + _, err = svc.NewKubeClient(svc.WithNamespace(openebsNamespace)).Update(newSvc) + if err != nil { + return fmt.Errorf("failed to update old service %s: %s", service.Name, err.Error()) + } } } } @@ -462,9 +513,21 @@ func preUpgrade(openebsNamespace string) error { for _, config := range webhookConfigList.Items { if config.Labels["openebs.io/version"] != version.Current() { - err = validate.KubeClient().Delete(config.Name, &metav1.DeleteOptions{}) - if err != nil { - return fmt.Errorf("failed to delete older webhook config %s: %s", config.Name, err.Error()) + if config.Labels["openebs.io/version"] == "" { + err = validate.KubeClient().Delete(config.Name, &metav1.DeleteOptions{}) + if err != nil { + return fmt.Errorf("failed to delete older webhook config %s: %s", config.Name, err.Error()) + } + } else { + newConfig := &config + for _, t := range transformConfig { + t(newConfig) + } + newConfig.Labels["openebs.io/version"] = version.Current() + _, err = validate.KubeClient().Update(newConfig) + if err != nil { + return fmt.Errorf("failed to update older webhook config %s: %s", config.Name, err.Error()) + } } } } From 69cd0907ce565721a06046e92056cbb339acedec Mon Sep 17 00:00:00 2001 From: shubham Date: Mon, 27 Jan 2020 16:15:07 +0530 Subject: [PATCH 4/9] fix scopelint issues Signed-off-by: shubham --- pkg/webhook/configuration.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/webhook/configuration.go b/pkg/webhook/configuration.go index 11e7e3038b..cc380299ce 100644 --- a/pkg/webhook/configuration.go +++ b/pkg/webhook/configuration.go @@ -468,12 +468,12 @@ func preUpgrade(openebsNamespace string) error { return fmt.Errorf("failed to delete old secret %s: %s", scrt.Name, err.Error()) } } else { - newScrt := &scrt + newScrt := scrt for _, t := range transformSecret { - t(newScrt) + t(&newScrt) } newScrt.Labels["openebs.io/version"] = version.Current() - _, err = secret.NewKubeClient(secret.WithNamespace(openebsNamespace)).Update(newScrt) + _, err = secret.NewKubeClient(secret.WithNamespace(openebsNamespace)).Update(&newScrt) if err != nil { return fmt.Errorf("failed to update old secret %s: %s", scrt.Name, err.Error()) } @@ -494,12 +494,12 @@ func preUpgrade(openebsNamespace string) error { return fmt.Errorf("failed to delete old service %s: %s", service.Name, err.Error()) } } else { - newSvc := &service + newSvc := service for _, t := range transformSvc { - t(newSvc) + t(&newSvc) } newSvc.Labels["openebs.io/version"] = version.Current() - _, err = svc.NewKubeClient(svc.WithNamespace(openebsNamespace)).Update(newSvc) + _, err = svc.NewKubeClient(svc.WithNamespace(openebsNamespace)).Update(&newSvc) if err != nil { return fmt.Errorf("failed to update old service %s: %s", service.Name, err.Error()) } @@ -519,12 +519,12 @@ func preUpgrade(openebsNamespace string) error { return fmt.Errorf("failed to delete older webhook config %s: %s", config.Name, err.Error()) } } else { - newConfig := &config + newConfig := config for _, t := range transformConfig { - t(newConfig) + t(&newConfig) } newConfig.Labels["openebs.io/version"] = version.Current() - _, err = validate.KubeClient().Update(newConfig) + _, err = validate.KubeClient().Update(&newConfig) if err != nil { return fmt.Errorf("failed to update older webhook config %s: %s", config.Name, err.Error()) } From 14f58ace256accb5e1cbcb552560a716424afbd7 Mon Sep 17 00:00:00 2001 From: shubham Date: Tue, 28 Jan 2020 15:44:30 +0530 Subject: [PATCH 5/9] fixed logs and changes literals to constants Signed-off-by: shubham --- pkg/kubernetes/secret/kubernetes.go | 2 +- pkg/kubernetes/service/v1alpha1/kubernetes.go | 2 +- pkg/webhook/configuration.go | 39 ++++++++++--------- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/pkg/kubernetes/secret/kubernetes.go b/pkg/kubernetes/secret/kubernetes.go index 18aef3ed11..063ee35671 100644 --- a/pkg/kubernetes/secret/kubernetes.go +++ b/pkg/kubernetes/secret/kubernetes.go @@ -219,7 +219,7 @@ func (k *Kubeclient) Update(secret *corev1.Secret) (*corev1.Secret, error) { } cli, err := k.getClientsetOrCached() if err != nil { - return nil, errors.Wrapf(err, "failed to update secret") + return nil, errors.Wrapf(err, "failed to update secret %s", secret.Name) } return k.update(cli, k.namespace, secret) } diff --git a/pkg/kubernetes/service/v1alpha1/kubernetes.go b/pkg/kubernetes/service/v1alpha1/kubernetes.go index 01b3a29c9b..0a2c01fc15 100644 --- a/pkg/kubernetes/service/v1alpha1/kubernetes.go +++ b/pkg/kubernetes/service/v1alpha1/kubernetes.go @@ -368,7 +368,7 @@ func (k *Kubeclient) Update(service *corev1.Service) (*corev1.Service, error) { if err != nil { return nil, errors.Wrapf( err, - "failed to create service {%s} in namespace {%s}", + "failed to update service {%s} in namespace {%s}", service.Name, service.Namespace, ) diff --git a/pkg/webhook/configuration.go b/pkg/webhook/configuration.go index cc380299ce..120f1684da 100644 --- a/pkg/webhook/configuration.go +++ b/pkg/webhook/configuration.go @@ -21,6 +21,7 @@ import ( "os" "strings" + apis "github.com/openebs/maya/pkg/apis/openebs.io/v1alpha1" menv "github.com/openebs/maya/pkg/env/v1alpha1" deploy "github.com/openebs/maya/pkg/kubernetes/deployment/appsv1/v1alpha1" secret "github.com/openebs/maya/pkg/kubernetes/secret" @@ -100,9 +101,9 @@ func createWebhookService( Namespace: namespace, Name: serviceName, Labels: map[string]string{ - "app": "admission-webhook", - "openebs.io/component-name": "admission-webhook-svc", - "openebs.io/version": version.GetVersion(), + "app": "admission-webhook", + "openebs.io/component-name": "admission-webhook-svc", + string(apis.OpenEBSVersionKey): version.GetVersion(), }, OwnerReferences: []metav1.OwnerReference{ownerReference}, }, @@ -192,9 +193,9 @@ func createAdmissionService( ObjectMeta: metav1.ObjectMeta{ Name: validatorWebhook, Labels: map[string]string{ - "app": "admission-webhook", - "openebs.io/component-name": "admission-webhook", - "openebs.io/version": version.GetVersion(), + "app": "admission-webhook", + "openebs.io/component-name": "admission-webhook", + string(apis.OpenEBSVersionKey): version.GetVersion(), }, OwnerReferences: []metav1.OwnerReference{ownerReference}, }, @@ -246,9 +247,9 @@ func createCertsSecret( Name: secretName, Namespace: namespace, Labels: map[string]string{ - "app": "admission-webhook", - "openebs.io/component-name": "admission-webhook", - "openebs.io/version": version.GetVersion(), + "app": "admission-webhook", + "openebs.io/component-name": "admission-webhook", + string(apis.OpenEBSVersionKey): version.GetVersion(), }, OwnerReferences: []metav1.OwnerReference{ownerReference}, }, @@ -430,7 +431,7 @@ type transformSecretFunc func(*corev1.Secret) type transformConfigFunc func(*v1beta1.ValidatingWebhookConfiguration) func addCSPCDeleteRule(config *v1beta1.ValidatingWebhookConfiguration) { - if config.Labels["openebs.io/version"] < "1.7.0" { + if config.Labels[string(apis.OpenEBSVersionKey)] < "1.7.0" { index := -1 for i, rule := range config.Webhooks[0].Rules { if util.ContainsString(rule.Rule.Resources, "cstorpoolclusters") { @@ -461,8 +462,8 @@ func preUpgrade(openebsNamespace string) error { } for _, scrt := range secretlist.Items { - if scrt.Labels["openebs.io/version"] != version.Current() { - if scrt.Labels["openebs.io/version"] == "" { + if scrt.Labels[string(apis.OpenEBSVersionKey)] != version.Current() { + if scrt.Labels[string(apis.OpenEBSVersionKey)] == "" { err = secret.NewKubeClient(secret.WithNamespace(openebsNamespace)).Delete(scrt.Name, &metav1.DeleteOptions{}) if err != nil { return fmt.Errorf("failed to delete old secret %s: %s", scrt.Name, err.Error()) @@ -472,7 +473,7 @@ func preUpgrade(openebsNamespace string) error { for _, t := range transformSecret { t(&newScrt) } - newScrt.Labels["openebs.io/version"] = version.Current() + newScrt.Labels[string(apis.OpenEBSVersionKey)] = version.Current() _, err = secret.NewKubeClient(secret.WithNamespace(openebsNamespace)).Update(&newScrt) if err != nil { return fmt.Errorf("failed to update old secret %s: %s", scrt.Name, err.Error()) @@ -487,8 +488,8 @@ func preUpgrade(openebsNamespace string) error { } for _, service := range svcList.Items { - if service.Labels["openebs.io/version"] != version.Current() { - if service.Labels["openebs.io/version"] == "" { + if service.Labels[string(apis.OpenEBSVersionKey)] != version.Current() { + if service.Labels[string(apis.OpenEBSVersionKey)] == "" { err = svc.NewKubeClient(svc.WithNamespace(openebsNamespace)).Delete(service.Name, &metav1.DeleteOptions{}) if err != nil { return fmt.Errorf("failed to delete old service %s: %s", service.Name, err.Error()) @@ -498,7 +499,7 @@ func preUpgrade(openebsNamespace string) error { for _, t := range transformSvc { t(&newSvc) } - newSvc.Labels["openebs.io/version"] = version.Current() + newSvc.Labels[string(apis.OpenEBSVersionKey)] = version.Current() _, err = svc.NewKubeClient(svc.WithNamespace(openebsNamespace)).Update(&newSvc) if err != nil { return fmt.Errorf("failed to update old service %s: %s", service.Name, err.Error()) @@ -512,8 +513,8 @@ func preUpgrade(openebsNamespace string) error { } for _, config := range webhookConfigList.Items { - if config.Labels["openebs.io/version"] != version.Current() { - if config.Labels["openebs.io/version"] == "" { + if config.Labels[string(apis.OpenEBSVersionKey)] != version.Current() { + if config.Labels[string(apis.OpenEBSVersionKey)] == "" { err = validate.KubeClient().Delete(config.Name, &metav1.DeleteOptions{}) if err != nil { return fmt.Errorf("failed to delete older webhook config %s: %s", config.Name, err.Error()) @@ -523,7 +524,7 @@ func preUpgrade(openebsNamespace string) error { for _, t := range transformConfig { t(&newConfig) } - newConfig.Labels["openebs.io/version"] = version.Current() + newConfig.Labels[string(apis.OpenEBSVersionKey)] = version.Current() _, err = validate.KubeClient().Update(&newConfig) if err != nil { return fmt.Errorf("failed to update older webhook config %s: %s", config.Name, err.Error()) From 30405ac920f4a2b2c003480effe7d15e3553e995 Mon Sep 17 00:00:00 2001 From: shubham Date: Tue, 28 Jan 2020 17:04:38 +0530 Subject: [PATCH 6/9] moved declaration of types and variables to the beginning of the file Signed-off-by: shubham --- pkg/webhook/configuration.go | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/pkg/webhook/configuration.go b/pkg/webhook/configuration.go index 120f1684da..522ee426fa 100644 --- a/pkg/webhook/configuration.go +++ b/pkg/webhook/configuration.go @@ -56,6 +56,10 @@ const ( rootCrt = "ca.crt" ) +type transformSvcFunc func(*corev1.Service) +type transformSecretFunc func(*corev1.Secret) +type transformConfigFunc func(*v1beta1.ValidatingWebhookConfiguration) + var ( // TimeoutSeconds specifies the timeout for this webhook. After the timeout passes, // the webhook call will be ignored or the API call will fail based on the @@ -64,6 +68,12 @@ var ( five = int32(5) // Ignore means that an error calling the webhook is ignored. Ignore = v1beta1.Ignore + // transformation function lists to upgrade webhook resources + transformSecret = []transformSecretFunc{} + transformSvc = []transformSvcFunc{} + transformConfig = []transformConfigFunc{ + addCSPCDeleteRule, + } ) // createWebhookService creates our webhook Service resource if it does not @@ -426,10 +436,6 @@ func GetAdmissionReference() (*metav1.OwnerReference, error) { return nil, fmt.Errorf("failed to create deployment ownerReference") } -type transformSvcFunc func(*corev1.Service) -type transformSecretFunc func(*corev1.Secret) -type transformConfigFunc func(*v1beta1.ValidatingWebhookConfiguration) - func addCSPCDeleteRule(config *v1beta1.ValidatingWebhookConfiguration) { if config.Labels[string(apis.OpenEBSVersionKey)] < "1.7.0" { index := -1 @@ -444,18 +450,9 @@ func addCSPCDeleteRule(config *v1beta1.ValidatingWebhookConfiguration) { } } -var ( - transformSecret = []transformSecretFunc{} - transformSvc = []transformSvcFunc{} - transformConfig = []transformConfigFunc{ - addCSPCDeleteRule, - } -) - // preUpgrade checks for the required older webhook configs,older // then 1.4.0 if exists delete them. func preUpgrade(openebsNamespace string) error { - secretlist, err := secret.NewKubeClient(secret.WithNamespace(openebsNamespace)).List(metav1.ListOptions{LabelSelector: webhookLabel}) if err != nil { return fmt.Errorf("failed to list old secret: %s", err.Error()) From 93515a00836bdf48ceacb31ce7efefa3e36318cc Mon Sep 17 00:00:00 2001 From: shubham Date: Tue, 28 Jan 2020 19:09:19 +0530 Subject: [PATCH 7/9] fix logs for svc and config Signed-off-by: shubham --- pkg/kubernetes/service/v1alpha1/kubernetes.go | 2 +- pkg/kubernetes/webhook/validate/v1alpha1/kubernetes.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/kubernetes/service/v1alpha1/kubernetes.go b/pkg/kubernetes/service/v1alpha1/kubernetes.go index 0a2c01fc15..bf992f98fb 100644 --- a/pkg/kubernetes/service/v1alpha1/kubernetes.go +++ b/pkg/kubernetes/service/v1alpha1/kubernetes.go @@ -362,7 +362,7 @@ func (k *Kubeclient) Create(service *corev1.Service) (*corev1.Service, error) { // Update updates a service in specified namespace in kubernetes cluster func (k *Kubeclient) Update(service *corev1.Service) (*corev1.Service, error) { if service == nil { - return nil, errors.New("failed to create service: nil service object") + return nil, errors.New("failed to update service: nil service object") } cli, err := k.getClientOrCached() if err != nil { diff --git a/pkg/kubernetes/webhook/validate/v1alpha1/kubernetes.go b/pkg/kubernetes/webhook/validate/v1alpha1/kubernetes.go index 8d5a132fcb..9bab76093c 100644 --- a/pkg/kubernetes/webhook/validate/v1alpha1/kubernetes.go +++ b/pkg/kubernetes/webhook/validate/v1alpha1/kubernetes.go @@ -214,7 +214,7 @@ func (k *Kubeclient) Delete(name string, options *metav1.DeleteOptions) error { // corresponding validatingWebhookConfiguration object, and an error if there is any. func (k *Kubeclient) Update(config *admission.ValidatingWebhookConfiguration) (*admission.ValidatingWebhookConfiguration, error) { if config == nil { - return nil, errors.New("failed to create validating configuration: nil configuration") + return nil, errors.New("failed to update validating configuration: nil configuration") } cs, err := k.getClientOrCached() if err != nil { From f74b5e2ad5b60cc4ec08a22b1d3e6bc08bc3b49b Mon Sep 17 00:00:00 2001 From: shubham Date: Wed, 29 Jan 2020 19:03:32 +0530 Subject: [PATCH 8/9] list for cvrs in all namespaces Signed-off-by: shubham --- pkg/webhook/cspc.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/webhook/cspc.go b/pkg/webhook/cspc.go index e66490673e..6d415e2ce4 100644 --- a/pkg/webhook/cspc.go +++ b/pkg/webhook/cspc.go @@ -145,7 +145,8 @@ func (wh *webhook) validateCSPCDeleteRequest(req *v1beta1.AdmissionRequest) *v1b return response } for _, cspiObj := range cspiList.Items { - cvrList, err := cvr.NewKubeclient().WithNamespace(cspiObj.Namespace).List(metav1.ListOptions{ + // list cvrs in all namespaces + cvrList, err := cvr.NewKubeclient().WithNamespace("").List(metav1.ListOptions{ LabelSelector: "cstorpoolinstance.openebs.io/name=" + cspiObj.Name, }) if err != nil { From 9a5cc80c8a37ed18c489cc590265d6a649baae48 Mon Sep 17 00:00:00 2001 From: shubham Date: Wed, 29 Jan 2020 20:02:46 +0530 Subject: [PATCH 9/9] added break in for loop and comments Signed-off-by: shubham --- pkg/webhook/configuration.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/webhook/configuration.go b/pkg/webhook/configuration.go index 522ee426fa..714958cfec 100644 --- a/pkg/webhook/configuration.go +++ b/pkg/webhook/configuration.go @@ -436,16 +436,24 @@ func GetAdmissionReference() (*metav1.OwnerReference, error) { return nil, fmt.Errorf("failed to create deployment ownerReference") } +// addCSPCDeleteRule adds the DELETE operation to for CSPC if coming from 1.6.0 +// or older version func addCSPCDeleteRule(config *v1beta1.ValidatingWebhookConfiguration) { if config.Labels[string(apis.OpenEBSVersionKey)] < "1.7.0" { index := -1 + // find the index of the RuleWithOperations having CSPC for i, rule := range config.Webhooks[0].Rules { if util.ContainsString(rule.Rule.Resources, "cstorpoolclusters") { index = i + break } } + // if CSPC RuleWithOperations is found append DELETE operation if index != -1 { - config.Webhooks[0].Rules[index].Operations = append(config.Webhooks[0].Rules[index].Operations, v1beta1.Delete) + config.Webhooks[0].Rules[index].Operations = append( + config.Webhooks[0].Rules[index].Operations, + v1beta1.Delete, + ) } } }