PWX-38596 : After disabling PX Security STC goes into DEGRADED state …

…due to rolling update failed (#1648)

* PWX-38596 : Skip adding token in context if security is disabled in stc

* update failing tests

drivers/storage/portworx/component/disruption_budget.go

@@ -90,7 +90,7 @@ func (c *disruptionBudget) Reconcile(cluster *corev1.StorageCluster) error {
 
 	// Get list of portworx storage nodes
 	nodeClient := api.NewOpenStorageNodeClient(c.sdkConn)
-	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, c.k8sClient, false)
+	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, c.k8sClient)
 	if err != nil {
 		return err
 	}

drivers/storage/portworx/component/security.go

@@ -534,7 +534,7 @@ func (c *security) updateSystemGuestRole(cluster *corev1.StorageCluster) error {
 	}
 
 	roleClient := api.NewOpenStorageRoleClient(c.sdkConn)
-	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, c.k8sClient, false)
+	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, c.k8sClient)
 	if err != nil {
 		c.closeSdkConn()
 		return err

drivers/storage/portworx/portworx.go

@@ -762,7 +762,7 @@ func (p *portworx) GetStorageNodes(
 	}
 
 	nodeClient := storageapi.NewOpenStorageNodeClient(p.sdkConn)
-	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, p.k8sClient, false)
+	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, p.k8sClient)
 	if err != nil {
 		return nil, err
 	}
@@ -787,10 +787,7 @@ func (p *portworx) GetKVDBMembers(cluster *corev1.StorageCluster) (map[string]bo
 	ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
 	defer cancel()
 
-	// if security was previously enabled and changed to disabled now
-	// the storagenodes might not be in sync since rolling update has not completed
-	// so always pass the token while rolling update
-	ctx, err = pxutil.SetupContextWithToken(ctx, cluster, p.k8sClient, true)
+	ctx, err = pxutil.SetupContextWithToken(ctx, cluster, p.k8sClient)
 	if err != nil {
 		return nil, err
 	}
@@ -815,7 +812,7 @@ func (p *portworx) GetNodesSelectedForUpgrade(cluster *corev1.StorageCluster, no
 		return nil, fmt.Errorf("failed to get Portworx connection: %v", err)
 	}
 	nodeClient := storageapi.NewOpenStorageNodeClient(p.sdkConn)
-	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, p.k8sClient, false)
+	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, p.k8sClient)
 	if err != nil {
 		return nil, fmt.Errorf("failed to setup context with token: %v", err)
 	}

drivers/storage/portworx/status.go

@@ -154,7 +154,7 @@ func (p *portworx) updatePortworxRuntimeStatus(
 	}
 
 	clusterClient := api.NewOpenStorageClusterClient(p.sdkConn)
-	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, p.k8sClient, false)
+	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, p.k8sClient)
 	if err != nil {
 		return err
 	}
@@ -419,7 +419,7 @@ func (p *portworx) updateStorageNodes(
 	cluster *corev1.StorageCluster,
 ) error {
 	nodeClient := api.NewOpenStorageNodeClient(p.sdkConn)
-	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, p.k8sClient, false)
+	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, p.k8sClient)
 	if err != nil {
 		return err
 	}

drivers/storage/portworx/status_test.go

@@ -62,7 +62,6 @@ func TestSetupContextWithToken(t *testing.T) {
 		initialSecrets    []v1.Secret
 		initialConfigMaps []v1.ConfigMap
 		securityEnabled   bool
-		skipSecurityCheck bool
 
 		expectError      bool
 		expectTokenAdded bool
@@ -73,7 +72,6 @@ func TestSetupContextWithToken(t *testing.T) {
 			pxSecretName:      "",
 			pxSharedSecretKey: "mysecret",
 			securityEnabled:   true,
-			skipSecurityCheck: false,
 
 			expectTokenAdded: true,
 			expectError:      false,
@@ -83,41 +81,19 @@ func TestSetupContextWithToken(t *testing.T) {
 			pxConfigMapName:   defaultConfigMap[0].Name,
 			initialConfigMaps: defaultConfigMap,
 			securityEnabled:   true,
-			skipSecurityCheck: false,
 
 			expectTokenAdded: true,
 			expectError:      false,
 		},
 		{
-			name:              "Default secret should generate and add token to context",
-			pxSecretName:      defaultSecret[0].Name,
-			initialSecrets:    defaultSecret,
-			securityEnabled:   true,
-			skipSecurityCheck: false,
-
-			expectTokenAdded: true,
-			expectError:      false,
-		},
-		{
-			name:              "Default secret should generate and add token to context when security is disabled but security check is skipped",
-			pxSecretName:      defaultSecret[0].Name,
-			initialSecrets:    defaultSecret,
-			securityEnabled:   false,
-			skipSecurityCheck: true,
+			name:            "Default secret should generate and add token to context",
+			pxSecretName:    defaultSecret[0].Name,
+			initialSecrets:  defaultSecret,
+			securityEnabled: true,
 
 			expectTokenAdded: true,
 			expectError:      false,
 		},
-		{
-			name:              "Default secret should not be generated when security is disabled and security check is not skipped",
-			pxSecretName:      defaultSecret[0].Name,
-			initialSecrets:    defaultSecret,
-			securityEnabled:   false,
-			skipSecurityCheck: false,
-
-			expectTokenAdded: false,
-			expectError:      false,
-		},
 	}
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
@@ -198,7 +174,7 @@ func TestSetupContextWithToken(t *testing.T) {
 			p := portworx{
 				k8sClient: k8sClient,
 			}
-			ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, p.k8sClient, tc.skipSecurityCheck)
+			ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, p.k8sClient)
 			if tc.expectError {
 				assert.Error(t, err)
 				assert.EqualError(t, err, tc.expectedError)

drivers/storage/portworx/util/util.go

@@ -79,8 +79,6 @@ const (
 	EssentialsOSBEndpointKey = "px-osb-endpoint"
 	// EnvKeyKubeletDir env var to set custom kubelet directory
 	EnvKeyKubeletDir = "KUBELET_DIR"
-	// Dummy Secret value for authentication when Security is disabled
-	DummySecretValue = "dummy-secret-value"
 
 	// AnnotationIsPKS annotation indicating whether it is a PKS cluster
 	AnnotationIsPKS = pxAnnotationPrefix + "/is-pks"
@@ -955,29 +953,26 @@ func SecurityEnabled(cluster *corev1.StorageCluster) bool {
 }
 
 // SetupContextWithToken Gets token or from secret for authenticating with the SDK server
-func SetupContextWithToken(ctx context.Context, cluster *corev1.StorageCluster, k8sClient client.Client, skipSecurityCheck bool) (context.Context, error) {
-	// auth not declared in cluster spec
-	// if security enabled check is skipped, proceed without returning context
-	if !SecurityEnabled(cluster) && !skipSecurityCheck {
-		return ctx, nil
-	}
+func SetupContextWithToken(ctx context.Context, cluster *corev1.StorageCluster, k8sClient client.Client) (context.Context, error) {
 
+	// Get the secret key for the system issuer
+	// return error if error is not NotFound
 	pxAppsSecret, err := GetSecretValue(ctx, cluster, k8sClient, SecurityPXSystemSecretsSecretName, SecurityAppsSecretKey)
-	if err != nil {
-		if !skipSecurityCheck {
-			return ctx, fmt.Errorf("failed to get portworx apps secret: %v", err.Error())
-		}
-		// if security enabled check is skipped and secret is not present, proceed with dummy secret value
-		// this is case of fresh install where security was never enabled
-		if errors.IsNotFound(err) && skipSecurityCheck {
-			pxAppsSecret = DummySecretValue
-		}
+	if err != nil && !errors.IsNotFound(err) {
+		return nil, err
 	}
 
-	if pxAppsSecret == "" {
+	// if secret is not found, check if security is enabled
+	if errors.IsNotFound(err) {
+		if SecurityEnabled(cluster) {
+			// when security is enabled in STC, and secret is not found, return error
+			return nil, fmt.Errorf("failed to get portworx apps secret: %v", err.Error())
+		}
+		// if security is not enabled in STC, it is okay to proceed without the secret
 		return ctx, nil
 	}
 
+	// security is enabled and secret is found
 	// Generate token and add to metadata.
 	name := "operator"
 	pxAppsIssuerVersion, err := version.NewVersion("2.6.0")
@@ -1127,7 +1122,7 @@ func CountStorageNodes(
 	nodeEnumerateResponse *api.SdkNodeEnumerateWithFiltersResponse,
 ) (int, error) {
 	nodeClient := api.NewOpenStorageNodeClient(sdkConn)
-	ctx, err := SetupContextWithToken(context.Background(), cluster, k8sClient, false)
+	ctx, err := SetupContextWithToken(context.Background(), cluster, k8sClient)
 	if err != nil {
 		return -1, err
 	}
@@ -1707,7 +1702,7 @@ func GetNodesToUpgrade(cluster *corev1.StorageCluster,
 
 	// Make a request to SDK API FilterNonOverlappingNodes to get output of list of nodes that can be upgraded in parallel
 	nodeClient := api.NewOpenStorageNodeClient(sdkConn)
-	ctx, err := SetupContextWithToken(context.Background(), cluster, k8sClient, false)
+	ctx, err := SetupContextWithToken(context.Background(), cluster, k8sClient)
 	if err != nil {
 		return nil, nil, -1, err
 	}

pkg/controller/storagenode/storagenode.go

@@ -510,7 +510,7 @@ func (c *Controller) isStorageNode(storageNode *corev1.StorageNode, cluster *cor
 	}
 
 	nodeClient := api.NewOpenStorageNodeClient(c.sdkConn)
-	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, c.client, false)
+	ctx, err := pxutil.SetupContextWithToken(context.Background(), cluster, c.client)
 	if err != nil {
 		logrus.Errorf("failed to setup context with token: %v", err)
 		return false, err