From 0b354413cc8f68b604b0507cee12cf4ff2ffb463 Mon Sep 17 00:00:00 2001 From: Moritz Johner Date: Wed, 7 Oct 2020 17:29:15 +0200 Subject: [PATCH] feat(aws): add region support to ssm and sm (#475) Signed-off-by: Moritz Johner --- README.md | 6 ++ e2e/tests/secrets-manager.test.js | 64 ++++++++++++++++++++ e2e/tests/ssm.test.js | 44 ++++++++++++++ examples/secretsmanager-example.yaml | 2 + examples/ssm-example.yaml | 2 + lib/backends/kv-backend.test.js | 15 +++-- lib/backends/secrets-manager-backend.js | 20 ++++-- lib/backends/secrets-manager-backend.test.js | 39 ++++++++++-- lib/backends/system-manager-backend.js | 20 ++++-- lib/backends/system-manager-backend.test.js | 42 +++++++++++-- 10 files changed, 232 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index e21f6d26..7d0b0a57 100644 --- a/README.md +++ b/README.md @@ -285,6 +285,8 @@ spec: backendType: secretsManager # optional: specify role to assume when retrieving the data roleArn: arn:aws:iam::123456789012:role/test-role + # optional: specify region + region: us-east-1 data: - key: hello-service/credentials name: password @@ -315,6 +317,8 @@ spec: backendType: secretsManager # optional: specify role to assume when retrieving the data roleArn: arn:aws:iam::123456789012:role/test-role + # optional: specify region + region: us-east-1 dataFrom: - hello-service/credentials ``` @@ -330,6 +334,8 @@ spec: backendType: secretsManager # optional: specify role to assume when retrieving the data roleArn: arn:aws:iam::123456789012:role/test-role + # optional: specify region + region: us-east-1 dataFrom: - hello-service/credentials data: diff --git a/e2e/tests/secrets-manager.test.js b/e2e/tests/secrets-manager.test.js index 043ee631..684bfb0c 100644 --- a/e2e/tests/secrets-manager.test.js +++ b/e2e/tests/secrets-manager.test.js @@ -124,6 +124,70 @@ describe('secretsmanager', async () => { expect(secret.body.type).to.equal('kubernetes.io/tls') }) + it('should pull existing secret from secretsmanager in the correct region', async () => { + const smEU = awsConfig.secretsManagerFactory({ + region: 'eu-west-1' + }) + const createSecret = util.promisify(smEU.createSecret).bind(smEU) + const putSecretValue = util.promisify(smEU.putSecretValue).bind(smEU) + + let result = await createSecret({ + Name: `e2e/${uuid}/x-region-credentials`, + SecretString: '{"username":"foo","password":"bar"}' + }).catch(err => { + expect(err).to.equal(null) + }) + + result = await kubeClient + .apis[customResourceManifest.spec.group] + .v1.namespaces('default')[customResourceManifest.spec.names.plural] + .post({ + body: { + apiVersion: 'kubernetes-client.io/v1', + kind: 'ExternalSecret', + metadata: { + name: `e2e-secretmanager-x-region-${uuid}` + }, + spec: { + backendType: 'secretsManager', + region: 'eu-west-1', + data: [ + { + key: `e2e/${uuid}/x-region-credentials`, + property: 'password', + name: 'password' + }, + { + key: `e2e/${uuid}/x-region-credentials`, + property: 'username', + name: 'username' + } + ] + } + } + }) + + expect(result).to.not.equal(undefined) + expect(result.statusCode).to.equal(201) + + let secret = await waitForSecret('default', `e2e-secretmanager-x-region-${uuid}`) + expect(secret).to.not.equal(undefined) + expect(secret.body.data.username).to.equal('Zm9v') + expect(secret.body.data.password).to.equal('YmFy') + + // update the secret value + result = await putSecretValue({ + SecretId: `e2e/${uuid}/x-region-credentials`, + SecretString: '{"username":"your mom","password":"1234"}' + }).catch(err => { + expect(err).to.equal(null) + }) + await delay(2000) + secret = await waitForSecret('default', `e2e-secretmanager-x-region-${uuid}`) + expect(secret.body.data.username).to.equal('eW91ciBtb20=') + expect(secret.body.data.password).to.equal('MTIzNA==') + }) + describe('permitted annotation', async () => { beforeEach(async () => { await kubeClient.api.v1.namespaces('default').patch({ diff --git a/e2e/tests/ssm.test.js b/e2e/tests/ssm.test.js index a2bc137b..23b76f70 100644 --- a/e2e/tests/ssm.test.js +++ b/e2e/tests/ssm.test.js @@ -53,6 +53,50 @@ describe('ssm', async () => { expect(secret.body.data.name).to.equal('Zm9v') }) + it('should pull existing secret from ssm in a different region', async () => { + const ssmEU = awsConfig.systemManagerFactory({ + region: 'eu-west-1' + }) + const putParameter = util.promisify(ssmEU.putParameter).bind(ssmEU) + + let result = await putParameter({ + Name: `/e2e/${uuid}/x-region`, + Type: 'String', + Value: 'foo' + }).catch(err => { + expect(err).to.equal(null) + }) + + result = await kubeClient + .apis[customResourceManifest.spec.group] + .v1.namespaces('default')[customResourceManifest.spec.names.plural] + .post({ + body: { + apiVersion: 'kubernetes-client.io/v1', + kind: 'ExternalSecret', + metadata: { + name: `e2e-ssm-xregion-${uuid}` + }, + spec: { + backendType: 'systemManager', + region: 'eu-west-1', + data: [ + { + key: `/e2e/${uuid}/x-region`, + name: 'name' + } + ] + } + } + }) + + expect(result).to.not.equal(undefined) + expect(result.statusCode).to.equal(201) + + const secret = await waitForSecret('default', `e2e-ssm-xregion-${uuid}`) + expect(secret.body.data.name).to.equal('Zm9v') + }) + describe('permitted annotation', async () => { beforeEach(async () => { await kubeClient.api.v1.namespaces('default').patch({ diff --git a/examples/secretsmanager-example.yaml b/examples/secretsmanager-example.yaml index 46f1b055..b79ef39f 100644 --- a/examples/secretsmanager-example.yaml +++ b/examples/secretsmanager-example.yaml @@ -6,6 +6,8 @@ spec: backendType: secretsManager # optional: specify role to assume when retrieving the data roleArn: arn:aws:iam::123412341234:role/let-other-account-access-secrets + # optional: specify region of the secret + region: eu-west-1 data: - key: demo-service/credentials name: password diff --git a/examples/ssm-example.yaml b/examples/ssm-example.yaml index be62b905..112bab03 100644 --- a/examples/ssm-example.yaml +++ b/examples/ssm-example.yaml @@ -6,6 +6,8 @@ spec: backendType: systemManager # optional: specify role to assume when retrieving the data roleArn: arn:aws:iam::123456789012:role/test-role + # optional: specify region + region: eu-west-1 data: - key: /foo/name1 name: variable-name diff --git a/lib/backends/kv-backend.test.js b/lib/backends/kv-backend.test.js index 39b01edb..6cb08437 100644 --- a/lib/backends/kv-backend.test.js +++ b/lib/backends/kv-backend.test.js @@ -296,7 +296,7 @@ describe('kv-backend', () => { expect(manifestData).deep.equals({}) }) - it('makes correct calls - with data and role', async () => { + it('makes correct calls - with data, role and region', async () => { await kvBackend.getSecretManifestData({ spec: { data: [ @@ -308,7 +308,8 @@ describe('kv-backend', () => { name: 'fakePropertyName2' } ], - roleArn: 'my-role' + roleArn: 'my-role', + region: 'my-region' } }) @@ -320,12 +321,18 @@ describe('kv-backend', () => { key: 'fakePropertyKey2', name: 'fakePropertyName2' }], - specOptions: { roleArn: 'my-role' } + specOptions: { + roleArn: 'my-role', + region: 'my-region' + } })).to.equal(true) expect(kvBackend._fetchDataFromValues.calledWith({ dataFrom: [], - specOptions: { roleArn: 'my-role' } + specOptions: { + roleArn: 'my-role', + region: 'my-region' + } })).to.equal(true) }) diff --git a/lib/backends/secrets-manager-backend.js b/lib/backends/secrets-manager-backend.js index 61f75a3d..4c1d88ec 100644 --- a/lib/backends/secrets-manager-backend.js +++ b/lib/backends/secrets-manager-backend.js @@ -26,20 +26,30 @@ class SecretsManagerBackend extends KVBackend { * @param {string} specOptions.roleArn - IAM role arn to assume * @returns {Promise} Promise object representing secret property value. */ - async _get ({ key, specOptions: { roleArn }, keyOptions: { versionStage = 'AWSCURRENT', versionId = null } }) { - this._logger.info(`fetching secret property ${key} with role: ${roleArn || 'pods role'}`) + async _get ({ key, specOptions: { roleArn, region }, keyOptions: { versionStage = 'AWSCURRENT', versionId = null } }) { + this._logger.info(`fetching secret property ${key} with role: ${roleArn || 'pods role'} in region ${region}`) let client = this._client + let factoryArgs = null if (roleArn) { const credentials = this._assumeRole({ RoleArn: roleArn, RoleSessionName: 'k8s-external-secrets' }) - client = this._clientFactory({ + factoryArgs = { + ...factoryArgs, credentials - }) + } + } + if (region) { + factoryArgs = { + ...factoryArgs, + region + } + } + if (factoryArgs) { + client = this._clientFactory(factoryArgs) } - let params if (versionId) { params = { SecretId: key, VersionId: versionId } diff --git a/lib/backends/secrets-manager-backend.test.js b/lib/backends/secrets-manager-backend.test.js index 2bd02790..023b0b36 100644 --- a/lib/backends/secrets-manager-backend.test.js +++ b/lib/backends/secrets-manager-backend.test.js @@ -81,19 +81,23 @@ describe('SecretsManagerBackend', () => { expect(secretPropertyValue.toString()).equals('fakeSecretPropertyValue') }) - it('returns secret property value assuming a role', async () => { + it('returns secret property value assuming a role with region', async () => { getSecretValuePromise.promise.resolves({ SecretString: 'fakeAssumeRoleSecretValue' }) const secretPropertyValue = await secretsManagerBackend._get({ key: 'fakeSecretKey', - specOptions: { roleArn: 'my-role' }, + specOptions: { + roleArn: 'my-role', + region: 'foo-bar-baz' + }, keyOptions }) expect(clientFactoryMock.lastArg).deep.equals({ - credentials: assumeRoleCredentials + credentials: assumeRoleCredentials, + region: 'foo-bar-baz' }) expect(clientMock.getSecretValue.calledWith({ SecretId: 'fakeSecretKey', @@ -101,12 +105,39 @@ describe('SecretsManagerBackend', () => { })).to.equal(true) expect(clientFactoryMock.getCall(0).args).deep.equals([]) expect(clientFactoryMock.getCall(1).args).deep.equals([{ - credentials: assumeRoleCredentials + credentials: assumeRoleCredentials, + region: 'foo-bar-baz' }]) expect(assumeRoleMock.callCount).equals(1) expect(secretPropertyValue).equals('fakeAssumeRoleSecretValue') }) + it('returns secret property value from specific region', async () => { + getSecretValuePromise.promise.resolves({ + SecretString: 'fakeAssumeRoleSecretValue' + }) + + const secretPropertyValue = await secretsManagerBackend._get({ + key: 'fakeSecretKey', + specOptions: { region: 'my-region' }, + keyOptions + }) + + expect(clientFactoryMock.lastArg).deep.equals({ + region: 'my-region' + }) + expect(clientMock.getSecretValue.calledWith({ + SecretId: 'fakeSecretKey', + VersionStage: 'AWSCURRENT' + })).to.equal(true) + expect(clientFactoryMock.getCall(0).args).deep.equals([]) + expect(clientFactoryMock.getCall(1).args).deep.equals([{ + region: 'my-region' + }]) + expect(assumeRoleMock.callCount).equals(0) + expect(secretPropertyValue).equals('fakeAssumeRoleSecretValue') + }) + it('returns secret property value with versionStage', async () => { getSecretValuePromise.promise.resolves({ SecretString: 'fakeSecretPropertyValuePreviousVersion' diff --git a/lib/backends/system-manager-backend.js b/lib/backends/system-manager-backend.js index 778cb58a..63b8b800 100644 --- a/lib/backends/system-manager-backend.js +++ b/lib/backends/system-manager-backend.js @@ -19,23 +19,33 @@ class SystemManagerBackend extends KVBackend { /** * Get secret property value from System Manager. * @param {string} key - Key used to store secret property value in System Manager. - * @param {object} keyOptions - Options for this specific key, eg version etc. * @param {object} specOptions - Options for this external secret, eg role * @param {string} specOptions.roleArn - IAM role arn to assume * @returns {Promise} Promise object representing secret property value. */ - async _get ({ key, specOptions: { roleArn } }) { - this._logger.info(`fetching secret property ${key} with role: ${roleArn || 'pods role'}`) + async _get ({ key, specOptions: { roleArn, region } }) { + this._logger.info(`fetching secret property ${key} with role: ${roleArn || 'pods role'} in region ${region}`) let client = this._client + let factoryArgs = null if (roleArn) { const credentials = this._assumeRole({ RoleArn: roleArn, RoleSessionName: 'k8s-external-secrets' }) - client = this._clientFactory({ + factoryArgs = { + ...factoryArgs, credentials - }) + } + } + if (region) { + factoryArgs = { + ...factoryArgs, + region + } + } + if (factoryArgs) { + client = this._clientFactory(factoryArgs) } try { const data = await client diff --git a/lib/backends/system-manager-backend.test.js b/lib/backends/system-manager-backend.test.js index aa86b84f..500e3b1a 100644 --- a/lib/backends/system-manager-backend.test.js +++ b/lib/backends/system-manager-backend.test.js @@ -63,7 +63,7 @@ describe('SystemManagerBackend', () => { expect(secretPropertyValue).equals('fakeSecretPropertyValue') }) - it('returns secret property value assuming a role', async () => { + it('returns secret property value assuming a role with region', async () => { getParameterPromise.promise.resolves({ Parameter: { Value: 'fakeAssumeRoleSecretValue' @@ -72,10 +72,43 @@ describe('SystemManagerBackend', () => { const secretPropertyValue = await systemManagerBackend._get({ key: 'fakeSecretKey', - specOptions: { roleArn: 'my-role' } + specOptions: { + roleArn: 'my-role', + region: 'my-region' + } + }) + expect(clientFactoryMock.lastArg).deep.equals({ + credentials: assumeRoleCredentials, + region: 'my-region' + }) + expect(clientMock.getParameter.calledWith({ + Name: 'fakeSecretKey', + WithDecryption: true + })).to.equal(true) + expect(clientFactoryMock.getCall(0).args).deep.equals([]) + expect(clientFactoryMock.getCall(1).args).deep.equals([{ + credentials: assumeRoleCredentials, + region: 'my-region' + }]) + expect(assumeRoleMock.callCount).equals(1) + expect(secretPropertyValue).equals('fakeAssumeRoleSecretValue') + }) + + it('returns secret property value from specific region', async () => { + getParameterPromise.promise.resolves({ + Parameter: { + Value: 'fakeAssumeRoleSecretValue' + } + }) + + const secretPropertyValue = await systemManagerBackend._get({ + key: 'fakeSecretKey', + specOptions: { + region: 'my-region' + } }) expect(clientFactoryMock.lastArg).deep.equals({ - credentials: assumeRoleCredentials + region: 'my-region' }) expect(clientMock.getParameter.calledWith({ Name: 'fakeSecretKey', @@ -83,9 +116,10 @@ describe('SystemManagerBackend', () => { })).to.equal(true) expect(clientFactoryMock.getCall(0).args).deep.equals([]) expect(clientFactoryMock.getCall(1).args).deep.equals([{ - credentials: assumeRoleCredentials + region: 'my-region' }]) expect(secretPropertyValue).equals('fakeAssumeRoleSecretValue') + expect(assumeRoleMock.callCount).equals(0) }) it('throws a meaningful message when the parameter does not exist', async () => {