From 7cc6e23a0b710173e7b46971c4633d6365040db1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Thu, 20 Jan 2022 14:43:21 +0100 Subject: [PATCH 1/2] osd: rename a few configuration flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For better consistency with IBM_KP_SERVICE_API_KEY and use "ibmkeyprotect" as a provider name for more clarity. Signed-off-by: Sébastien Han --- .github/workflows/canary-integration-test.yml | 2 +- .github/workflows/daily-nightly-jobs.yml | 2 +- .../workflows/encryption-pvc-kms-ibm-kp/action.yml | 10 +++++----- Documentation/ceph-kms.md | 4 ++-- pkg/apis/ceph.rook.io/v1/security.go | 2 +- pkg/daemon/ceph/osd/kms/envs_test.go | 12 ++++++------ pkg/daemon/ceph/osd/kms/ibm_key_protect.go | 8 ++++---- pkg/daemon/ceph/osd/kms/ibm_key_protect_test.go | 2 +- pkg/daemon/ceph/osd/kms/kms.go | 8 ++++---- pkg/daemon/ceph/osd/kms/kms_test.go | 6 +++--- tests/manifests/test-kms-ibm-kp-spec.in | 4 ++-- 11 files changed, 30 insertions(+), 30 deletions(-) diff --git a/.github/workflows/canary-integration-test.yml b/.github/workflows/canary-integration-test.yml index fc1ea7be8..2111c1a6b 100644 --- a/.github/workflows/canary-integration-test.yml +++ b/.github/workflows/canary-integration-test.yml @@ -840,7 +840,7 @@ 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 != ''" with: ibm-instance-id: ${{ secrets.IBM_INSTANCE_ID }} ibm-service-api-key: ${{ secrets.IBM_SERVICE_API_KEY }} diff --git a/.github/workflows/daily-nightly-jobs.yml b/.github/workflows/daily-nightly-jobs.yml index 7c7df6e28..3458b4391 100644 --- a/.github/workflows/daily-nightly-jobs.yml +++ b/.github/workflows/daily-nightly-jobs.yml @@ -326,7 +326,7 @@ 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 != ''" with: github-token: ${{ secrets.GITHUB_TOKEN }} ibm-instance-id: ${{ secrets.IBM_KP_INSTANCE_ID }} diff --git a/.github/workflows/encryption-pvc-kms-ibm-kp/action.yml b/.github/workflows/encryption-pvc-kms-ibm-kp/action.yml index 4b83110a8..ee8fdc6db 100644 --- a/.github/workflows/encryption-pvc-kms-ibm-kp/action.yml +++ b/.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 diff --git a/Documentation/ceph-kms.md b/Documentation/ceph-kms.md index 24f0d373b..24f84d553 100644 --- a/Documentation/ceph-kms.md +++ b/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: + KMS_PROVIDER: ibmkeyprotect + IBM_KP_SERVICE_INSTANCE_ID: # name of the k8s secret containing the service API Key tokenSecretName: ibm-kp-svc-api-key ``` diff --git a/pkg/apis/ceph.rook.io/v1/security.go b/pkg/apis/ceph.rook.io/v1/security.go index 01f678898..135c97e0b 100644 --- a/pkg/apis/ceph.rook.io/v1/security.go +++ b/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 diff --git a/pkg/daemon/ceph/osd/kms/envs_test.go b/pkg/daemon/ceph/osd/kms/envs_test.go index aafb8b312..a8c562f93 100644 --- a/pkg/daemon/ceph/osd/kms/envs_test.go +++ b/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}, }, }, } diff --git a/pkg/daemon/ceph/osd/kms/ibm_key_protect.go b/pkg/daemon/ceph/osd/kms/ibm_key_protect.go index 675a5380d..c673372f4 100644 --- a/pkg/daemon/ceph/osd/kms/ibm_key_protect.go +++ b/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 } diff --git a/pkg/daemon/ceph/osd/kms/ibm_key_protect_test.go b/pkg/daemon/ceph/osd/kms/ibm_key_protect_test.go index 5a19b164a..aeeaeb948 100644 --- a/pkg/daemon/ceph/osd/kms/ibm_key_protect_test.go +++ b/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) diff --git a/pkg/daemon/ceph/osd/kms/kms.go b/pkg/daemon/ceph/osd/kms/kms.go index ab005c2bf..cf1734284 100644 --- a/pkg/daemon/ceph/osd/kms/kms.go +++ b/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) diff --git a/pkg/daemon/ceph/osd/kms/kms_test.go b/pkg/daemon/ceph/osd/kms/kms_test.go index 180f9b32b..459d8718b 100644 --- a/pkg/daemon/ceph/osd/kms/kms_test.go +++ b/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) { diff --git a/tests/manifests/test-kms-ibm-kp-spec.in b/tests/manifests/test-kms-ibm-kp-spec.in index 4070971a8..6bbebd77d 100644 --- a/tests/manifests/test-kms-ibm-kp-spec.in +++ b/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 From 802a86ccd5013da977695b91cc4d9b91db0ca53c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Han?= Date: Fri, 21 Jan 2022 10:19:21 +0100 Subject: [PATCH 2/2] ci: fix encryption test condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the "if" to work we need to bring the env in the step with the "env" keyword. Otherwise, the env is never imported and the condition is always false. Signed-off-by: Sébastien Han --- .github/workflows/canary-integration-test.yml | 5 ++++- .github/workflows/daily-nightly-jobs.yml | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/canary-integration-test.yml b/.github/workflows/canary-integration-test.yml index 2111c1a6b..9aa1c7d30 100644 --- a/.github/workflows/canary-integration-test.yml +++ b/.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_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 }} diff --git a/.github/workflows/daily-nightly-jobs.yml b/.github/workflows/daily-nightly-jobs.yml index 3458b4391..8318b8278 100644 --- a/.github/workflows/daily-nightly-jobs.yml +++ b/.github/workflows/daily-nightly-jobs.yml @@ -327,6 +327,9 @@ jobs: - name: run Encryption KMS IBM Key Protect uses: ./.github/workflows/encryption-pvc-kms-ibm-kp 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 }}