Merge pull request #9615 from rook/ibm-kms-followup

osd: rename a few configuration flags

.github/workflows/canary-integration-test.yml

@@ -838,9 +838,12 @@ jobs:
       with:
         fetch-depth: 0
 
-    - name: run Encryption KMS IBM Key Protect
+    - name: run encryption KMS IBM Key Protect
       uses: ./.github/workflows/encryption-pvc-kms-ibm-kp
-      if: "env.IBM_KP_INSTANCE_ID != '' && env.IBM_KP_SERVICE_API_KEY != ''"
+      if: "env.IBM_KP_SERVICE_INSTANCE_ID != '' && env.IBM_KP_SERVICE_API_KEY != ''"
+      env:
+        IBM_KP_SERVICE_INSTANCE_ID: ${{ secrets.IBM_INSTANCE_ID }}
+        IBM_KP_SERVICE_API_KEY: ${{ secrets.IBM_SERVICE_API_KEY }}
       with:
         ibm-instance-id: ${{ secrets.IBM_INSTANCE_ID }}
         ibm-service-api-key: ${{ secrets.IBM_SERVICE_API_KEY }}

.github/workflows/daily-nightly-jobs.yml

@@ -326,7 +326,10 @@ jobs:
 
     - name: run Encryption KMS IBM Key Protect
       uses: ./.github/workflows/encryption-pvc-kms-ibm-kp
-      if: "env.IBM_KP_INSTANCE_ID != '' && env.IBM_KP_SERVICE_API_KEY != ''"
+      if: "env.IBM_KP_SERVICE_INSTANCE_ID != '' && env.IBM_KP_SERVICE_API_KEY != ''"
+      env:
+        IBM_KP_SERVICE_INSTANCE_ID: ${{ secrets.IBM_INSTANCE_ID }}
+        IBM_KP_SERVICE_API_KEY: ${{ secrets.IBM_SERVICE_API_KEY }}
       with:
         github-token: ${{ secrets.GITHUB_TOKEN }}
         ibm-instance-id: ${{ secrets.IBM_KP_INSTANCE_ID }}

.github/workflows/encryption-pvc-kms-ibm-kp/action.yml

@@ -2,7 +2,7 @@ name: Encryption KMS IBM Key Protect
 description: Reusable workflow to test Encryption KMS IBM Key Protect
 inputs:
   ibm-instance-id:
-    description: IBM_KP_INSTANCE_ID from the calling workflow
+    description: IBM_KP_SERVICE_INSTANCE_ID from the calling workflow
     required: true
   ibm-service-api-key:
     description: IBM_KP_SERVICE_API_KEY from the calling workflow
@@ -15,12 +15,12 @@ runs:
   using: "composite"
   steps:
     - name: fail if env no present
-      if: "env.IBM_KP_INSTANCE_ID == '' || env.IBM_KP_SERVICE_API_KEY == ''"
+      if: "env.IBM_KP_SERVICE_INSTANCE_ID == '' || env.IBM_KP_SERVICE_API_KEY == ''"
       env:
-        IBM_KP_INSTANCE_ID: ${{ inputs.ibm-instance-id }}
+        IBM_KP_SERVICE_INSTANCE_ID: ${{ inputs.ibm-instance-id }}
         IBM_KP_SERVICE_API_KEY: ${{ inputs.ibm-service-api-key }}
       shell: bash --noprofile --norc -eo pipefail -x {0}
-      run: echo "IBM_KP_INSTANCE_ID and IBM_KP_SERVICE_API_KEY must be set in the environment" && exit 0
+      run: echo "IBM_KP_SERVICE_INSTANCE_ID and IBM_KP_SERVICE_API_KEY must be set in the environment" && exit 0
 
     - name: setup cluster resources
       uses: ./.github/workflows/setup-cluster-resources
@@ -42,7 +42,7 @@ runs:
     - name: deploy cluster
       shell: bash --noprofile --norc -eo pipefail -x {0}
       env:
-        IBM_KP_INSTANCE_ID: ${{ inputs.ibm-instance-id }}
+        IBM_KP_SERVICE_INSTANCE_ID: ${{ inputs.ibm-instance-id }}
         IBM_KP_SERVICE_API_KEY: ${{ inputs.ibm-service-api-key }}
       run: |
         tests/scripts/github-action-helper.sh deploy_manifest_with_local_build deploy/examples/operator.yaml

Documentation/ceph-kms.md

@@ -266,8 +266,8 @@ security:
   kms:
     # name of the k8s config map containing all the kms connection details
     connectionDetails:
-      KMS_PROVIDER: ibm-kp
-      IBM_KP_INSTANCE_ID: <instance ID that was retrieved in the first paragraph>
+      KMS_PROVIDER: ibmkeyprotect
+      IBM_KP_SERVICE_INSTANCE_ID: <instance ID that was retrieved in the first paragraph>
     # name of the k8s secret containing the service API Key
     tokenSecretName: ibm-kp-svc-api-key
 ```

pkg/apis/ceph.rook.io/v1/security.go

@@ -50,7 +50,7 @@ func (kms *KeyManagementServiceSpec) IsVaultKMS() bool {
 
 // IsIBMKeyProtectKMS return whether IBM Key Protect KMS is configured
 func (kms *KeyManagementServiceSpec) IsIBMKeyProtectKMS() bool {
-	return getParam(kms.ConnectionDetails, "KMS_PROVIDER") == secrets.TypeIBM
+	return getParam(kms.ConnectionDetails, "KMS_PROVIDER") == "ibmkeyprotect"
 }
 
 // IsTLSEnabled return KMS TLS details are configured

pkg/daemon/ceph/osd/kms/envs_test.go

@@ -63,7 +63,7 @@ func TestVaultTLSEnvVarFromSecret(t *testing.T) {
 	t.Run("ibm kp", func(t *testing.T) {
 		spec := cephv1.ClusterSpec{Security: cephv1.SecuritySpec{KeyManagementService: cephv1.KeyManagementServiceSpec{
 			TokenSecretName:   "ibm-kp-token",
-			ConnectionDetails: map[string]string{"KMS_PROVIDER": "ibm-kp", "IBM_KP_INSTANCE_ID": "1"}}},
+			ConnectionDetails: map[string]string{"KMS_PROVIDER": TypeIBM, "IBM_KP_SERVICE_INSTANCE_ID": "1"}}},
 		}
 		envVars := ConfigToEnvVar(spec)
 		areEnvVarsSorted := sort.SliceIsSorted(envVars, func(i, j int) bool {
@@ -71,8 +71,8 @@ func TestVaultTLSEnvVarFromSecret(t *testing.T) {
 		})
 		assert.True(t, areEnvVarsSorted)
 		assert.Equal(t, 3, len(envVars))
-		assert.Contains(t, envVars, v1.EnvVar{Name: "KMS_PROVIDER", Value: "ibm-kp"})
-		assert.Contains(t, envVars, v1.EnvVar{Name: "IBM_KP_INSTANCE_ID", Value: "1"})
+		assert.Contains(t, envVars, v1.EnvVar{Name: "KMS_PROVIDER", Value: TypeIBM})
+		assert.Contains(t, envVars, v1.EnvVar{Name: "IBM_KP_SERVICE_INSTANCE_ID", Value: "1"})
 		assert.Contains(t, envVars, v1.EnvVar{Name: "IBM_KP_SERVICE_API_KEY", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "ibm-kp-token"}, Key: "IBM_KP_SERVICE_API_KEY"}}})
 	})
 }
@@ -140,11 +140,11 @@ func TestVaultConfigToEnvVar(t *testing.T) {
 		},
 		{
 			"ibm kp - IBM_KP_SERVICE_API_KEY is removed from the details",
-			args{spec: cephv1.ClusterSpec{Security: cephv1.SecuritySpec{KeyManagementService: cephv1.KeyManagementServiceSpec{ConnectionDetails: map[string]string{"KMS_PROVIDER": "ibm-kp", "IBM_KP_SERVICE_API_KEY": "foo", "IBM_KP_INSTANCE_ID": "1"}, TokenSecretName: "ibm-kp-token"}}}},
+			args{spec: cephv1.ClusterSpec{Security: cephv1.SecuritySpec{KeyManagementService: cephv1.KeyManagementServiceSpec{ConnectionDetails: map[string]string{"KMS_PROVIDER": TypeIBM, "IBM_KP_SERVICE_API_KEY": "foo", "IBM_KP_SERVICE_INSTANCE_ID": "1"}, TokenSecretName: "ibm-kp-token"}}}},
 			[]v1.EnvVar{
-				{Name: "IBM_KP_INSTANCE_ID", Value: "1"},
 				{Name: "IBM_KP_SERVICE_API_KEY", ValueFrom: &v1.EnvVarSource{SecretKeyRef: &v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: "ibm-kp-token"}, Key: "IBM_KP_SERVICE_API_KEY"}}},
-				{Name: "KMS_PROVIDER", Value: "ibm-kp"},
+				{Name: "IBM_KP_SERVICE_INSTANCE_ID", Value: "1"},
+				{Name: "KMS_PROVIDER", Value: TypeIBM},
 			},
 		},
 	}

pkg/daemon/ceph/osd/kms/ibm_key_protect.go

@@ -18,15 +18,15 @@ package kms
 
 import (
 	kp "github.com/IBM/keyprotect-go-client"
-	"github.com/libopenstorage/secrets"
 	"github.com/pkg/errors"
 )
 
 const (
+	TypeIBM = "ibmkeyprotect"
 	// IbmKeyProtectServiceApiKey is the IBM Key Protect service API key
 	IbmKeyProtectServiceApiKey = "IBM_KP_SERVICE_API_KEY"
 	// IbmKeyProtectInstanceIdKey is the IBM Key Protect instance id
-	IbmKeyProtectInstanceIdKey = "IBM_KP_INSTANCE_ID"
+	IbmKeyProtectInstanceIdKey = "IBM_KP_SERVICE_INSTANCE_ID"
 	// IbmKeyProtectBaseUrlKey is the IBM Key Protect base url
 	IbmKeyProtectBaseUrlKey = "IBM_KP_BASE_URL"
 	// IbmKeyProtectTokenUrlKey is the IBM Key Protect token url
@@ -39,7 +39,7 @@ var (
 	kmsIBMKeyProtectMandatoryConnectionDetails = []string{IbmKeyProtectInstanceIdKey, IbmKeyProtectServiceApiKey}
 	// ErrIbmServiceApiKeyNotSet is returned when IBM_KP_SERVICE_API_KEY is not set
 	ErrIbmServiceApiKeyNotSet = errors.Errorf("%s not set.", IbmKeyProtectServiceApiKey)
-	// ErrIbmInstanceIdKeyNotSet is returned when IBM_KP_INSTANCE_ID is not set
+	// ErrIbmInstanceIdKeyNotSet is returned when IBM_KP_SERVICE_INSTANCE_ID is not set
 	ErrIbmInstanceIdKeyNotSet = errors.Errorf("%s not set.", IbmKeyProtectInstanceIdKey)
 )
 
@@ -83,4 +83,4 @@ func InitKeyProtect(config map[string]string) (*kp.Client, error) {
 }
 
 // IsIBMKeyProtect determines whether the configured KMS is IBM Key Protect
-func (c *Config) IsIBMKeyProtect() bool { return c.Provider == secrets.TypeIBM }
+func (c *Config) IsIBMKeyProtect() bool { return c.Provider == TypeIBM }

pkg/daemon/ceph/osd/kms/ibm_key_protect_test.go

@@ -35,7 +35,7 @@ func TestInitKeyProtect(t *testing.T) {
 		config[IbmKeyProtectServiceApiKey] = "foo"
 	})
 
-	t.Run("IBM_KP_INSTANCE_ID not set", func(t *testing.T) {
+	t.Run("IBM_KP_SERVICE_INSTANCE_ID not set", func(t *testing.T) {
 		_, err := InitKeyProtect(config)
 		assert.Error(t, err)
 		assert.ErrorIs(t, err, ErrIbmInstanceIdKeyNotSet)

pkg/daemon/ceph/osd/kms/kms.go

@@ -65,8 +65,8 @@ func NewConfig(context *clusterd.Context, clusterSpec *cephv1.ClusterSpec, clust
 		config.Provider = secrets.TypeK8s
 	case secrets.TypeVault:
 		config.Provider = secrets.TypeVault
-	case secrets.TypeIBM:
-		config.Provider = secrets.TypeIBM
+	case TypeIBM:
+		config.Provider = TypeIBM
 	default:
 		logger.Errorf("unsupported kms type %q", Provider)
 	}
@@ -248,7 +248,7 @@ func ValidateConnectionDetails(ctx context.Context, clusterdContext *clusterd.Co
 				return errors.Wrap(err, "failed to set vault kms token to an env var")
 			}
 
-		case secrets.TypeIBM:
+		case TypeIBM:
 			for _, config := range kmsIBMKeyProtectMandatoryTokenDetails {
 				v, ok := kmsToken.Data[config]
 				if !ok || len(v) == 0 {
@@ -281,7 +281,7 @@ func ValidateConnectionDetails(ctx context.Context, clusterdContext *clusterd.Co
 			}
 		}
 
-	case secrets.TypeIBM:
+	case TypeIBM:
 		for _, config := range kmsIBMKeyProtectMandatoryConnectionDetails {
 			if GetParam(securitySpec.KeyManagementService.ConnectionDetails, config) == "" {
 				return errors.Errorf("failed to validate kms config %q. cannot be empty", config)

pkg/daemon/ceph/osd/kms/kms_test.go

@@ -58,7 +58,7 @@ func TestValidateConnectionDetails(t *testing.T) {
 	}
 	ibmSecuritySpec := &cephv1.SecuritySpec{KeyManagementService: cephv1.KeyManagementServiceSpec{
 		ConnectionDetails: map[string]string{
-			"KMS_PROVIDER": "ibm-kp",
+			"KMS_PROVIDER": TypeIBM,
 		},
 	}}
 
@@ -183,8 +183,8 @@ func TestValidateConnectionDetails(t *testing.T) {
 		assert.NoError(t, err)
 		err = ValidateConnectionDetails(ctx, context, ibmSecuritySpec, ns)
 		assert.Error(t, err, "")
-		assert.EqualError(t, err, "failed to validate kms config \"IBM_KP_INSTANCE_ID\". cannot be empty")
-		ibmSecuritySpec.KeyManagementService.ConnectionDetails["IBM_KP_INSTANCE_ID"] = "foo"
+		assert.EqualError(t, err, "failed to validate kms config \"IBM_KP_SERVICE_INSTANCE_ID\". cannot be empty")
+		ibmSecuritySpec.KeyManagementService.ConnectionDetails["IBM_KP_SERVICE_INSTANCE_ID"] = "foo"
 	})
 
 	t.Run("ibm kp - success", func(t *testing.T) {

tests/manifests/test-kms-ibm-kp-spec.in

@@ -2,6 +2,6 @@ spec:
   security:
     kms:
       connectionDetails:
-        KMS_PROVIDER: ibm-kp
-        IBM_KP_INSTANCE_ID: $IBM_KP_INSTANCE_ID
+        KMS_PROVIDER: ibmkeyprotect
+        IBM_KP_SERVICE_INSTANCE_ID: $IBM_KP_SERVICE_INSTANCE_ID
       tokenSecretName: rook-ibm-kp-token