Fix the issues that devops credentials cannot be deleted

Signed-off-by: rick <1450685+LinuxSuRen@users.noreply.github.com>

pkg/apis/devops/v1alpha3/credential_types.go

@@ -25,6 +25,7 @@ This file will not contain CRD, but the credential type constants and their fiel
 const (
 	CredentialFinalizerName  = "finalizers.kubesphere.io/credential"
 	DevOpsCredentialPrefix   = "credential.devops.kubesphere.io/"
+	DevOpsCredentialDataHash = DevOpsCredentialPrefix + "datahash"
 	// SecretTypeBasicAuth contains data needed for basic authentication.
 	//
 	// Required at least one of fields:

pkg/controller/devopscredential/devopscredential_controller.go

@@ -19,6 +19,7 @@ package devopscredential
 import (
 	"context"
 	"fmt"
+	"github.com/emicklei/go-restful"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -36,10 +37,12 @@ import (
 	devopsv1alpha3 "kubesphere.io/kubesphere/pkg/apis/devops/v1alpha3"
 	kubesphereclient "kubesphere.io/kubesphere/pkg/client/clientset/versioned"
 	"kubesphere.io/kubesphere/pkg/constants"
+	"kubesphere.io/kubesphere/pkg/controller/utils"
 	modelsdevops "kubesphere.io/kubesphere/pkg/models/devops"
 	devopsClient "kubesphere.io/kubesphere/pkg/simple/client/devops"
 	"kubesphere.io/kubesphere/pkg/utils/k8sutil"
 	"kubesphere.io/kubesphere/pkg/utils/sliceutil"
+	"net/http"
 	"reflect"
 	"strings"
 	"time"
@@ -218,7 +221,7 @@ func (c *Controller) syncHandler(key string) error {
 		return err
 	}
 	if !isDevOpsProjectAdminNamespace(namespace) {
-		err := fmt.Errorf("cound not create credential in normal namespaces %s", namespace.Name)
+		err := fmt.Errorf("cound not create or update credential '%s' in normal namespaces %s", name, namespace.Name)
 		klog.Warning(err)
 		return err
 	}
@@ -233,14 +236,26 @@ func (c *Controller) syncHandler(key string) error {
 		return err
 	}
 
-	//If the sync is successful, return handle
-	if state, ok := secret.Annotations[devopsv1alpha3.CredentialSyncStatusAnnoKey]; ok && state == modelsdevops.StatusSuccessful {
-		return nil
-	}
-
 	copySecret := secret.DeepCopy()
 	// DeletionTimestamp.IsZero() means copySecret has not been deleted.
-	if secret.ObjectMeta.DeletionTimestamp.IsZero() {
+	if copySecret.ObjectMeta.DeletionTimestamp.IsZero() {
+		// make sure Annotations is not nil
+		if copySecret.Annotations == nil {
+			copySecret.Annotations = map[string]string{}
+		}
+
+		//If the sync is successful, return handle
+		if state, ok := copySecret.Annotations[devopsv1alpha3.CredentialSyncStatusAnnoKey]; ok && state == modelsdevops.StatusSuccessful {
+			specHash := utils.ComputeHash(copySecret.Data)
+			oldHash, _ := copySecret.Annotations[devopsv1alpha3.DevOpsCredentialDataHash] // don't need to check if it's nil, only compare if they're different
+			if specHash == oldHash {
+				// it was synced successfully, and there's any change with the Pipeline spec, skip this round
+				return nil
+			} else {
+				copySecret.Annotations[devopsv1alpha3.DevOpsCredentialDataHash] = specHash
+			}
+		}
+
 		// https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#finalizers
 		if !sliceutil.HasString(secret.ObjectMeta.Finalizers, devopsv1alpha3.CredentialFinalizerName) {
 			copySecret.ObjectMeta.Finalizers = append(copySecret.ObjectMeta.Finalizers, devopsv1alpha3.CredentialFinalizerName)
@@ -268,13 +283,31 @@ func (c *Controller) syncHandler(key string) error {
 	} else {
 		// Finalizers processing logic
 		if sliceutil.HasString(copySecret.ObjectMeta.Finalizers, devopsv1alpha3.CredentialFinalizerName) {
+			delSuccess := false
 			if _, err := c.devopsClient.DeleteCredentialInProject(nsName, secret.Name); err != nil {
-				klog.V(8).Info(err, fmt.Sprintf("failed to delete secret %s in devops", key))
+				// the status code should be 404 if the credential does not exists
-				return err
+				if srvErr, ok := err.(restful.ServiceError); ok {
+					delSuccess = srvErr.Code == http.StatusNotFound
+				} else if srvErr, ok := err.(*devopsClient.ErrorResponse); ok {
+					delSuccess = srvErr.Response.StatusCode == http.StatusNotFound
+				} else {
+					klog.Error(fmt.Sprintf("unexpected error type: %v, should be *restful.ServiceError", err))
 				}
+
+				klog.V(8).Info(err, fmt.Sprintf("failed to delete secret %s in devops", key))
+			} else {
+				delSuccess = true
+			}
+
+			if delSuccess {
 				copySecret.ObjectMeta.Finalizers = sliceutil.RemoveString(copySecret.ObjectMeta.Finalizers, func(item string) bool {
 					return item == devopsv1alpha3.CredentialFinalizerName
 				})
+			} else {
+				// make sure the corresponding Jenkins credentials can be clean
+				// You can remove the finalizer via kubectl manually in a very special case that Jenkins might be not able to available anymore
+				return fmt.Errorf("failed to remove devops credential finalizer due to bad communication with Jenkins")
+			}
 
 		}
 	}

pkg/controller/pipeline/pipeline_controller.go

@@ -19,14 +19,10 @@ package pipeline
 import (
 	"context"
 	"fmt"
-	"github.com/davecgh/go-spew/spew"
 	"github.com/emicklei/go-restful"
-	"hash"
-	"hash/fnv"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/rand"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apimachinery/pkg/util/wait"
 	corev1informer "k8s.io/client-go/informers/core/v1"
@@ -43,6 +39,7 @@ import (
 	devopsinformers "kubesphere.io/kubesphere/pkg/client/informers/externalversions/devops/v1alpha3"
 	devopslisters "kubesphere.io/kubesphere/pkg/client/listers/devops/v1alpha3"
 	"kubesphere.io/kubesphere/pkg/constants"
+	"kubesphere.io/kubesphere/pkg/controller/utils"
 	modelsdevops "kubesphere.io/kubesphere/pkg/models/devops"
 	devopsClient "kubesphere.io/kubesphere/pkg/simple/client/devops"
 	"kubesphere.io/kubesphere/pkg/utils/k8sutil"
@@ -240,7 +237,7 @@ func (c *Controller) syncHandler(key string) error {
 
 		//If the sync is successful, return handle
 		if state, ok := copyPipeline.Annotations[devopsv1alpha3.PipelineSyncStatusAnnoKey]; ok && state == modelsdevops.StatusSuccessful {
-			specHash := computeHash(copyPipeline.Spec)
+			specHash := utils.ComputeHash(copyPipeline.Spec)
 			oldHash, _ := copyPipeline.Annotations[devopsv1alpha3.PipelineSpecHash] // don't need to check if it's nil, only compare if they're different
 			if specHash == oldHash {
 				// it was synced successfully, and there's any change with the Pipeline spec, skip this round
@@ -319,30 +316,6 @@ func (c *Controller) syncHandler(key string) error {
 	return nil
 }
 
-func computeHash(obj interface{}) string {
-	hasher := fnv.New32a()
-	deepHashObject(hasher, obj)
-	return rand.SafeEncodeString(fmt.Sprint(hasher.Sum32()))
-}
-
-// deepHashObject writes specified object to hash using the spew library
-// which follows pointers and prints actual values of the nested objects
-// ensuring the hash does not change when a pointer changes.
-// **Notice**
-// we don't want to import k8s.io/kubernetes as a module, but this is a very small function
-// so just copy it from k8s.io/kubernetes@v1.14.0/pkg/util/hash/hash.go
-// **Notice End**
-func deepHashObject(hasher hash.Hash, objectToWrite interface{}) {
-	hasher.Reset()
-	printer := spew.ConfigState{
-		Indent:         " ",
-		SortKeys:       true,
-		DisableMethods: true,
-		SpewKeys:       true,
-	}
-	printer.Fprintf(hasher, "%#v", objectToWrite)
-}
-
 func isDevOpsProjectAdminNamespace(namespace *v1.Namespace) bool {
 	_, ok := namespace.Labels[constants.DevOpsProjectLabelKey]
 

pkg/controller/utils/hash.go

@@ -0,0 +1,34 @@
+package utils
+
+import (
+	"fmt"
+	"github.com/davecgh/go-spew/spew"
+	"hash"
+	"hash/fnv"
+	"k8s.io/apimachinery/pkg/util/rand"
+)
+
+// ComputeHash computes hash value of a interface
+func ComputeHash(obj interface{}) string {
+	hasher := fnv.New32a()
+	deepHashObject(hasher, obj)
+	return rand.SafeEncodeString(fmt.Sprint(hasher.Sum32()))
+}
+
+// deepHashObject writes specified object to hash using the spew library
+// which follows pointers and prints actual values of the nested objects
+// ensuring the hash does not change when a pointer changes.
+// **Notice**
+// we don't want to import k8s.io/kubernetes as a module, but this is a very small function
+// so just copy it from k8s.io/kubernetes@v1.14.0/pkg/util/hash/hash.go
+// **Notice End**
+func deepHashObject(hasher hash.Hash, objectToWrite interface{}) {
+	hasher.Reset()
+	printer := spew.ConfigState{
+		Indent:         " ",
+		SortKeys:       true,
+		DisableMethods: true,
+		SpewKeys:       true,
+	}
+	printer.Fprintf(hasher, "%#v", objectToWrite)
+}