Merge pull request noobaa#1797 from liranmauda/liran-backport-into-5.17

liranmauda · web-flow · commit 38baece0560c · 2026-02-04T20:59:11.000+02:00
[Direct to 5.17] Reverting kms changes
diff --git a/.github/workflows/run_hac_test.yml b/.github/workflows/run_hac_test.yml
@@ -13,7 +13,13 @@ jobs:
         uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version-file: go.mod
+          check-latest: true
+          cache: true
+          cache-dependency-path: |
+            **/go.sum
+      - name: Set up kind
+        uses: helm/kind-action@v1
 
       - name: Set environment variables
         run: |
diff --git a/.github/workflows/run_kms_azure_vault_test.yml b/.github/workflows/run_kms_azure_vault_test.yml
@@ -13,7 +13,13 @@ jobs:
         uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version-file: go.mod
+          check-latest: true
+          cache: true
+          cache-dependency-path: |
+            **/go.sum
+      - name: Set up kind
+        uses: helm/kind-action@v1
 
       - name: Set environment variables
         run: |
diff --git a/.github/workflows/run_kms_dev_test.yml b/.github/workflows/run_kms_dev_test.yml
@@ -13,7 +13,13 @@ jobs:
         uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version-file: go.mod
+          check-latest: true
+          cache: true
+          cache-dependency-path: |
+            **/go.sum
+      - name: Set up kind
+        uses: helm/kind-action@v1
 
       - name: Set environment variables
         run: |
diff --git a/.github/workflows/run_kms_ibm_kp_test.yml b/.github/workflows/run_kms_ibm_kp_test.yml
@@ -16,7 +16,13 @@ jobs:
         uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version-file: go.mod
+          check-latest: true
+          cache: true
+          cache-dependency-path: |
+            **/go.sum
+      - name: Set up kind
+        uses: helm/kind-action@v1
 
       - name: Set environment variables
         run: |
diff --git a/.github/workflows/run_kms_kmip_test.yml b/.github/workflows/run_kms_kmip_test.yml
@@ -13,22 +13,28 @@ jobs:
         uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version-file: go.mod
+          check-latest: true
+          cache: true
+          cache-dependency-path: |
+            **/go.sum
+      - name: Set up kind
+        uses: helm/kind-action@v1
 
       - name: Set environment variables
         run: |
           echo PATH=$PATH:$HOME/go/bin                                          >> $GITHUB_ENV
           echo OPERATOR_IMAGE=localhost:5000/noobaa/noobaa-operator:integration >> $GITHUB_ENV
           echo PYKMIP_IMAGE=localhost:5000/noobaa/pykmip:integration >> $GITHUB_ENV
-      
+
       - name: Deploy Dependencies
         run: |
           set -x
           bash .travis/install-5nodes-kind-cluster.sh
           go get -v github.com/onsi/ginkgo/ginkgo
           go install -v github.com/onsi/ginkgo/ginkgo
           ginkgo version
-          
+
       - name: Build NooBaa
         run: |
           make cli
diff --git a/.github/workflows/run_kms_rotate_test.yml b/.github/workflows/run_kms_rotate_test.yml
@@ -13,7 +13,13 @@ jobs:
         uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version-file: go.mod
+          check-latest: true
+          cache: true
+          cache-dependency-path: |
+            **/go.sum
+      - name: Set up kind
+        uses: helm/kind-action@v1
 
       - name: Set environment variables
         run: |
diff --git a/.github/workflows/run_kms_tls_sa_test.yml b/.github/workflows/run_kms_tls_sa_test.yml
@@ -13,8 +13,13 @@ jobs:
         uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
-
+          go-version-file: go.mod
+          check-latest: true
+          cache: true
+          cache-dependency-path: |
+            **/go.sum
+      - name: Set up kind
+        uses: helm/kind-action@v1
       - name: Set environment variables
         run: |
           echo PATH=$PATH:$HOME/go/bin                                          >> $GITHUB_ENV
diff --git a/.github/workflows/run_kms_tls_token_test.yml b/.github/workflows/run_kms_tls_token_test.yml
@@ -13,7 +13,13 @@ jobs:
         uses: actions/checkout@v4
       - uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version-file: go.mod
+          check-latest: true
+          cache: true
+          cache-dependency-path: |
+            **/go.sum
+      - name: Set up kind
+        uses: helm/kind-action@v1
 
       - name: Set environment variables
         run: |
diff --git a/deploy/internal/deployment-endpoint.yaml b/deploy/internal/deployment-endpoint.yaml
@@ -116,6 +116,7 @@ spec:
             - name: ENDPOINT_GROUP_ID
             - name: LOCAL_MD_SERVER
             - name: LOCAL_N2N_AGENT
+            - name: NOOBAA_ROOT_SECRET
             - name: NODE_EXTRA_CA_CERTS
             - name: GUARANTEED_LOGS_PATH
             - name: CONTAINER_CPU_REQUEST
diff --git a/deploy/internal/statefulset-core.yaml b/deploy/internal/statefulset-core.yaml
@@ -115,6 +115,7 @@ spec:
               value: postgres
             - name: CONTAINER_PLATFORM
               value: KUBERNETES
+            - name: NOOBAA_ROOT_SECRET
             - name: NODE_EXTRA_CA_CERTS
             - name: AGENT_PROFILE
               value: VALUE_AGENT_PROFILE
diff --git a/pkg/bundle/deploy.go b/pkg/bundle/deploy.go
@@ -3900,7 +3900,7 @@ data:
     pg_stat_statements.track = all
 `
 
-const Sha256_deploy_internal_deployment_endpoint_yaml = "4e512a82e4af6176fb9d76fbf4b630ca6399df50062e52ec5cd6692ebae525b4"
+const Sha256_deploy_internal_deployment_endpoint_yaml = "0784d71f1a50b8b2f216adb957ea4ce90392e39981bd584dd5e98272327a99c2"
 
 const File_deploy_internal_deployment_endpoint_yaml = `apiVersion: apps/v1
 kind: Deployment
@@ -4020,6 +4020,7 @@ spec:
             - name: ENDPOINT_GROUP_ID
             - name: LOCAL_MD_SERVER
             - name: LOCAL_N2N_AGENT
+            - name: NOOBAA_ROOT_SECRET
             - name: NODE_EXTRA_CA_CERTS
             - name: GUARANTEED_LOGS_PATH
             - name: CONTAINER_CPU_REQUEST
@@ -4959,7 +4960,7 @@ spec:
       noobaa-s3-svc: "true"
 `
 
-const Sha256_deploy_internal_statefulset_core_yaml = "729f2a3c02c44b2001f5f5e99e4a0db26d100a85ad3de75f6c2c48551094952e"
+const Sha256_deploy_internal_statefulset_core_yaml = "447d0c9d6831eb9074e8648609614268430b4d0f89d618a4c9a250053f858290"
 
 const File_deploy_internal_statefulset_core_yaml = `apiVersion: apps/v1
 kind: StatefulSet
@@ -5078,6 +5079,7 @@ spec:
               value: postgres
             - name: CONTAINER_PLATFORM
               value: KUBERNETES
+            - name: NOOBAA_ROOT_SECRET
             - name: NODE_EXTRA_CA_CERTS
             - name: AGENT_PROFILE
               value: VALUE_AGENT_PROFILE
diff --git a/pkg/system/phase2_creating.go b/pkg/system/phase2_creating.go
@@ -359,21 +359,6 @@ func (r *Reconciler) SetDesiredNooBaaDB() error {
 }
 
 func (r *Reconciler) setDesiredCoreEnv(c *corev1.Container) {
-	// Filter out NOOBAA_ROOT_SECRET from env vars to avoid exposing it in pod spec
-	// it is set via mounting the secret as files
-	// this will remove the leftover env var in case of an upgrade from older operator version (older than 4.21)
-	// as we preserve env vars on updates by merging the arrays and not replacing them.
-
-    if len(c.Env) > 0 {
-        filtered := c.Env[:0]
-        for _, env := range c.Env {
-            if env.Name != "NOOBAA_ROOT_SECRET" {
-                filtered = append(filtered, env)
-            }
-        }
-        c.Env = filtered
-    }
-
 	for j := range c.Env {
 		switch c.Env[j].Name {
 		case "AGENT_PROFILE":
@@ -446,6 +431,8 @@ func (r *Reconciler) setDesiredCoreEnv(c *corev1.Container) {
 			if r.NooBaa.Spec.ExternalPgSSLUnauthorized {
 				c.Env[j].Value = "true"
 			}
+		case "NOOBAA_ROOT_SECRET":
+			c.Env[j].Value = r.SecretRootMasterKey
 		case "NODE_EXTRA_CA_CERTS":
 			c.Env[j].Value = r.ApplyCAsToPods
 		case "GUARANTEED_LOGS_PATH":
diff --git a/pkg/system/phase4_configuring.go b/pkg/system/phase4_configuring.go
@@ -371,6 +371,8 @@ func (r *Reconciler) SetDesiredDeploymentEndpoint() error {
 					if r.JoinSecret == nil {
 						c.Env[j].Value = "true"
 					}
+				case "NOOBAA_ROOT_SECRET":
+					c.Env[j].Value = r.SecretRootMasterKey
 				case "VIRTUAL_HOSTS":
 					hosts := []string{}
 					for _, addr := range r.NooBaa.Status.Services.ServiceS3.InternalDNS {
diff --git a/pkg/util/kms/kms_kmip.go b/pkg/util/kms/kms_kmip.go
@@ -14,8 +14,6 @@ const (
 	KMIPEndpoint      = "KMIP_ENDPOINT"
 	KMIPSecret        = "KMIP_CERTS_SECRET"
 	KMIPUniqueID      = "UniqueIdentifier"
-	NewKMIPUniqueID   = "UniqueIdentifierNew"
-	NewActiveKeyID    = "NewActiveKeyID"
 	KMIPTLSServerName = "TLS_SERVER_NAME"
 	KMIPReadTimeOut   = "READ_TIMEOUT"
 	KMIPWriteTimeOut  = "WRITE_TIMEOUT"
@@ -27,9 +25,6 @@ const (
 
 // KMIP is a kmip driver
 type KMIP struct {
-	UID  string // NooBaa system UID
-	name string // NooBaa system name
-	ns   string // NooBaa system namespace
 }
 
 // NewKMIP is KMIP driver constructor
@@ -38,7 +33,7 @@ func NewKMIP(
 	namespace string,
 	uid string,
 ) Driver {
-	return &KMIP{uid, name, namespace}
+	return &KMIP{}
 }
 
 //
@@ -113,7 +108,7 @@ func (k *KMIP) DeleteContext() map[string]string {
 // Version returns the current driver KMS version
 // either single string or map, i.e. rotating key
 func (k *KMIP) Version(kms *KMS) Version {
-	return &VersionRotatingSecret{VersionBase{kms, nil}, k.name, k.ns}
+	return &VersionSingleSecret{kms, nil}
 }
 
 // Register KMIP driver with KMS layer
diff --git a/pkg/util/kms/kms_kmip_storage.go b/pkg/util/kms/kms_kmip_storage.go
@@ -4,7 +4,6 @@ import (
 	"bufio"
 	"bytes"
 	"fmt"
-	"strings"
 	"time"
 
 	"crypto/tls"
@@ -210,7 +209,7 @@ func (k *KMIPSecretStorage) response(respMsg *kmip.ResponseMessage, operation km
 		return nil, fmt.Errorf("Unexpected uniqueBatchItemID, real %v expected %v", bi.UniqueBatchItemID, uniqueBatchItemID)
 	}
 	if kmip14.ResultStatusSuccess != bi.ResultStatus {
-		return nil, fmt.Errorf("Unexpected result status %v: Reason: %v Message: %v", bi.ResultStatus, bi.ResultReason, bi.ResultMessage)
+		return nil, fmt.Errorf("Unexpected result status %v expected success %v", bi.ResultStatus, kmip14.ResultStatusSuccess)
 	}
 
 	return &bi, nil
@@ -262,23 +261,10 @@ func (k *KMIPSecretStorage) GetSecret(
 
 	log := util.Logger()
 
-	lookfor := KMIPUniqueID // Addition to upgrade
-	var activeKeyID string
-	util.KubeCheck(k.secret)
-	if strings.HasSuffix(secretID, "-root-master-key-backend") {
-		lookfor = NewKMIPUniqueID
-		exists := false
-		activeKeyID, exists = k.secret.StringData[NewActiveKeyID]
-		if !exists {
-			log.Errorf("KMIPSecretStorage.GetSecret() activeKeyID %v does not exist in secret %v", activeKeyID, k.secret.Name)
-			return nil, secrets.NoVersion, secrets.ErrInvalidSecretId
-		}
-	}
-
 	// KMIP key uniqueIdentifier
-	uniqueIdentifier, exists := k.secret.StringData[lookfor]
+	uniqueIdentifier, exists := k.secret.StringData[KMIPUniqueID]
 	if !exists {
-		log.Errorf("KMIPSecretStorage.GetSecret() uniqueIdentifier %v does not exist in secret %v", lookfor, k.secret.Name)
+		log.Errorf("KMIPSecretStorage.GetSecret() uniqueIdentifier %v does not exist in secret %v", KMIPUniqueID, k.secret)
 		return nil, secrets.NoVersion, secrets.ErrInvalidSecretId
 	}
 
@@ -316,26 +302,23 @@ func (k *KMIPSecretStorage) GetSecret(
 	if getRespPayload.SymmetricKey == nil {
 		return nil, secrets.NoVersion, fmt.Errorf("Unexpected  get response SymmetricKey can not be nil")
 	}
-	if getRespPayload.SymmetricKey.KeyBlock.CryptographicLength != cryptographicLength {
-		return nil, secrets.NoVersion, fmt.Errorf("Unexpected  KeyBlock crypto len actual %v, expected %v", getRespPayload.SymmetricKey.KeyBlock.CryptographicLength, cryptographicLength)
-	}
 	if getRespPayload.SymmetricKey.KeyBlock.KeyFormatType != kmip14.KeyFormatTypeRaw {
 		return nil, secrets.NoVersion, fmt.Errorf("Unexpected  KeyBlock format type actual %v, expected KeyFormatTypeRaw %v", getRespPayload.SymmetricKey.KeyBlock.KeyFormatType, kmip14.KeyFormatTypeRaw)
 	}
+	if getRespPayload.SymmetricKey.KeyBlock.CryptographicLength != cryptographicLength {
+		return nil, secrets.NoVersion, fmt.Errorf("Unexpected  KeyBlock crypto len actual %v, expected %v", getRespPayload.SymmetricKey.KeyBlock.CryptographicLength, cryptographicLength)
+	}
 	if getRespPayload.SymmetricKey.KeyBlock.CryptographicAlgorithm != kmip14.CryptographicAlgorithmAES {
 		return nil, secrets.NoVersion, fmt.Errorf("Unexpected  KeyBlock crypto algo actual %v, expected CryptographicAlgorithmAES %v", getRespPayload.SymmetricKey.KeyBlock.CryptographicAlgorithm, kmip14.CryptographicAlgorithmAES)
 	}
 
 	secretBytes := getRespPayload.SymmetricKey.KeyBlock.KeyValue.KeyMaterial.([]byte)
 	secretBase64 := base64.StdEncoding.EncodeToString(secretBytes)
 
-	if len(activeKeyID) > 0 {
-		r := map[string]interface{}{ActiveRootKey: activeKeyID, activeKeyID: secretBase64}
-		return r, secrets.NoVersion, nil
-	} else {
-		r := map[string]interface{}{secretID: secretBase64}
-		return r, secrets.NoVersion, nil
-	}
+	// Return the fetched key value
+	r := map[string]interface{}{secretID: secretBase64}
+
+	return r, secrets.NoVersion, nil
 }
 
 // PutSecret will associate an secretId to its secret data
@@ -346,10 +329,13 @@ func (k *KMIPSecretStorage) PutSecret(
 	keyContext map[string]string,
 ) (secrets.Version, error) {
 	log := util.Logger()
+	if _, exists := k.secret.StringData[KMIPUniqueID]; exists {
+		log.Errorf("KMIPSecretStorage.PutSecret() Key UniqueIdentifier %v was not found in the secret", KMIPUniqueID)
+		return secrets.NoVersion, secrets.ErrSecretExists
+	}
 
 	// Register the key value the KMIP endpoint
-	activeKey := plainText[ActiveRootKey].(string)
-	value := plainText[activeKey].(string)
+	value := plainText[secretID].(string)
 	valueBytes, err := base64.StdEncoding.DecodeString(value)
 	if err != nil {
 		return secrets.NoVersion, err
@@ -394,8 +380,7 @@ func (k *KMIPSecretStorage) PutSecret(
 		return secrets.NoVersion, err
 	}
 
-	k.secret.StringData[NewActiveKeyID] = activeKey
-	k.secret.StringData[NewKMIPUniqueID] = registerRespPayload.UniqueIdentifier
+	k.secret.StringData[KMIPUniqueID] = registerRespPayload.UniqueIdentifier
 	if !util.KubeUpdate(k.secret) {
 		log.Errorf("Failed to update KMS secret %v in ns %v", k.secret.Name, k.secret.Namespace)
 		return secrets.NoVersion, fmt.Errorf("Failed to update KMS secret %v in ns %v", k.secret.Name, k.secret.Namespace)
@@ -411,15 +396,10 @@ func (k *KMIPSecretStorage) DeleteSecret(
 ) error {
 	log := util.Logger()
 
-	lookfor := KMIPUniqueID // Addition to upgrade
-	if strings.HasSuffix(secretID, "-root-master-key-backend") {
-		lookfor = NewKMIPUniqueID
-	}
 	// Find the key ID
-	util.KubeCheck(k.secret)
-	uniqueIdentifier, exists := k.secret.StringData[lookfor]
+	uniqueIdentifier, exists := k.secret.StringData[KMIPUniqueID]
 	if !exists {
-		log.Errorf("KMIPSecretStorage.DeleteSecret() No uniqueIdentifier %v in the secret %v", lookfor, k.secret.Name)
+		log.Errorf("KMIPSecretStorage.DeleteSecret() No uniqueIdentifier in the secret")
 		return secrets.ErrInvalidSecretId
 	}
 
@@ -457,8 +437,8 @@ func (k *KMIPSecretStorage) DeleteSecret(
 		return fmt.Errorf("Unexpected uniqueIdentifier %v in destroy response , expected %v", destroyRespPayload.UniqueIdentifier, uniqueIdentifier)
 	}
 
-	delete(k.secret.Data, lookfor)
-	delete(k.secret.StringData, lookfor)
+	delete(k.secret.Data, KMIPUniqueID)
+	delete(k.secret.StringData, KMIPUniqueID)
 	if !util.KubeUpdate(k.secret) {
 		log.Errorf("Failed to update KMS secret %v in ns %v", k.secret.Name, k.secret.Namespace)
 		return fmt.Errorf("Failed to update KMS secret %v in ns %v", k.secret.Name, k.secret.Namespace)
diff --git a/pkg/util/kms/kms_version.go b/pkg/util/kms/kms_version.go
diff --git a/pkg/util/kms/test/kmip/kms_kmip_test.go b/pkg/util/kms/test/kmip/kms_kmip_test.go
