Skip to content
This repository has been archived by the owner on Sep 9, 2023. It is now read-only.

Commit

Permalink
feat: allow enforcing naming conventions for key names, limiting whic…
Browse files Browse the repository at this point in the history
…h keys can be fetched from backends (external-secrets#230)

* feat(naming-convention): enforcing naming conventions for AWS Secrets Manager entries

closes external-secrets#178

* chore: fix typo namspace to namespace

* chore(naming-convention): early return on isPermitted

also improved wording for naming permitted annotation

related external-secrets#178

* docs(naming-convention): initial documentation

related external-secrets#178
  • Loading branch information
muenchhausen authored and Flydiverny committed Dec 9, 2019
1 parent 234e907 commit c4fdea6
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 7 deletions.
26 changes: 26 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,32 @@ data:
password: MTIzNA==
```

## Enforcing naming conventions for backend keys

by default an `ExternalSecret` may access arbitrary keys from the backend e.g.

```yml
data:
- key: /dev/cluster1/core-namespace/hello-service/password
name: password
```

An enforced naming convention helps to keep the structure tidy and limits the access according
to your naming schema.

Configure the schema as regular expression in the namespace using an annotation.
This allows `ExternalSecrets` in `core-namespace` just to access secrets that start with
`/dev/cluster1/core-namespace/`:

```yaml
kind: Namespace
metadata:
name: core-namespace
annotations:
# annotation key is configurable
externalsecrets.kubernetes-client.io/permitted-key-name: "/dev/cluster1/core-namespace/.*"
```

## Deprecations

A few properties has changed name overtime, we still maintain backwards compatbility with these but they will eventually be removed, and they are not validated using the CRD validation.
Expand Down
4 changes: 3 additions & 1 deletion bin/daemon.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ const {
metricsPort,
pollerIntervalMilliseconds,
pollingDisabled,
rolePermittedAnnotation
rolePermittedAnnotation,
namingPermittedAnnotation
} = require('../config')

async function main () {
Expand All @@ -49,6 +50,7 @@ async function main () {
metrics,
pollerIntervalMilliseconds,
rolePermittedAnnotation,
namingPermittedAnnotation,
customResourceManifest,
pollingDisabled,
logger
Expand Down
2 changes: 2 additions & 0 deletions config/environment.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const logLevel = process.env.LOG_LEVEL || 'info'
const pollingDisabled = 'DISABLE_POLLING' in process.env

const rolePermittedAnnotation = process.env.ROLE_PERMITTED_ANNOTATION || 'iam.amazonaws.com/permitted'
const namingPermittedAnnotation = process.env.NAMING_PERMITTED_ANNOTATION || 'externalsecrets.kubernetes-client.io/permitted-key-name'

const metricsPort = process.env.METRICS_PORT || 3001

Expand All @@ -33,6 +34,7 @@ module.exports = {
pollerIntervalMilliseconds,
metricsPort,
rolePermittedAnnotation,
namingPermittedAnnotation,
pollingDisabled,
logLevel
}
2 changes: 1 addition & 1 deletion e2e/tests/secrets-manager.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ describe('secretsmanager', async () => {
.externalsecrets(`e2e-secretmanager-permitted-tls-${uuid}`)
.get()
expect(result).to.not.equal(undefined)
expect(result.body.status.status).to.contain('namspace does not allow to assume role let-me-be-root')
expect(result.body.status.status).to.contain('namespace does not allow to assume role let-me-be-root')
})
})
})
2 changes: 1 addition & 1 deletion e2e/tests/ssm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ describe('ssm', async () => {
.externalsecrets(`e2e-ssm-permitted-${uuid}`)
.get()
expect(result).to.not.equal(undefined)
expect(result.body.status.status).to.contain('namspace does not allow to assume role let-me-be-root')
expect(result.body.status.status).to.contain('namespace does not allow to assume role let-me-be-root')
})
})
})
3 changes: 3 additions & 0 deletions lib/poller-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class PollerFactory {
metrics,
pollerIntervalMilliseconds,
rolePermittedAnnotation,
namingPermittedAnnotation,
customResourceManifest,
pollingDisabled,
logger
Expand All @@ -30,6 +31,7 @@ class PollerFactory {
this._pollerIntervalMilliseconds = pollerIntervalMilliseconds
this._customResourceManifest = customResourceManifest
this._rolePermittedAnnotation = rolePermittedAnnotation
this._namingPermittedAnnotation = namingPermittedAnnotation
this._pollingDisabled = pollingDisabled
}

Expand All @@ -46,6 +48,7 @@ class PollerFactory {
metrics: this._metrics,
customResourceManifest: this._customResourceManifest,
rolePermittedAnnotation: this._rolePermittedAnnotation,
namingPermittedAnnotation: this._namingPermittedAnnotation,
pollingDisabled: this._pollingDisabled,
externalSecret
})
Expand Down
36 changes: 33 additions & 3 deletions lib/poller.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class Poller {
metrics,
customResourceManifest,
rolePermittedAnnotation,
namingPermittedAnnotation,
pollingDisabled,
externalSecret
}) {
Expand All @@ -49,6 +50,7 @@ class Poller {
this._metrics = metrics
this._pollingDisabled = pollingDisabled
this._rolePermittedAnnotation = rolePermittedAnnotation
this._namingPermittedAnnotation = namingPermittedAnnotation
this._customResourceManifest = customResourceManifest

this._externalSecret = externalSecret
Expand Down Expand Up @@ -187,21 +189,49 @@ class Poller {
* @param {Object} descriptor secret descriptor
*/
_isPermitted (namespace, descriptor) {
const role = descriptor.roleArn
let allowed = true
let reason = ''

if (!namespace.metadata.annotations || !role) {
if (!namespace.metadata.annotations) {
return {
allowed, reason
}
}

// 1. testing naming convention if defined in namespace

const externalData = descriptor.data || descriptor.properties
const namingConvention = namespace.metadata.annotations[this._namingPermittedAnnotation]

if (namingConvention) {
externalData.forEach((secretProperty, index) => {
const reNaming = new RegExp(namingConvention)
if (!reNaming.test(secretProperty.key)) {
allowed = false
reason = `key name does not match naming convention ${namingConvention}`
return {
allowed, reason
}
}
})
}

// 2. testing assume role if configured

const role = descriptor.roleArn

if (!role) {
return {
allowed, reason
}
}

// an empty annotation value allows access to all roles
const re = new RegExp(namespace.metadata.annotations[this._rolePermittedAnnotation])

if (!re.test(role)) {
allowed = false
reason = `namspace does not allow to assume role ${role}`
reason = `namespace does not allow to assume role ${role}`
}

return {
Expand Down
62 changes: 61 additions & 1 deletion lib/poller.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('Poller', () => {
})

const rolePermittedAnnotation = 'iam.amazonaws.com/permitted'
const namingPermittedAnnotation = 'externalsecrets.kubernetes-client.io/permitted-key-name'

beforeEach(() => {
backendMock = sinon.mock()
Expand Down Expand Up @@ -93,6 +94,7 @@ describe('Poller', () => {
logger: loggerMock,
externalSecret: fakeExternalSecret,
rolePermittedAnnotation,
namingPermittedAnnotation,
customResourceManifest: fakeCustomResourceManifest
})
}
Expand Down Expand Up @@ -636,7 +638,7 @@ describe('Poller', () => {
}

expect(error).to.not.equal(undefined)
expect(error.message).equals('not allowed to fetch secret: fakeNamespace/fakeSecretName: namspace does not allow to assume role arn:aws:iam::123456789012:role/test-role')
expect(error.message).equals('not allowed to fetch secret: fakeNamespace/fakeSecretName: namespace does not allow to assume role arn:aws:iam::123456789012:role/test-role')
})

it('fails storing secret', async () => {
Expand Down Expand Up @@ -745,6 +747,64 @@ describe('Poller', () => {
}
]

for (let i = 0; i < testcases.length; i++) {
const testcase = testcases[i]
const verdict = poller._isPermitted(testcase.ns, testcase.descriptor)
expect(verdict.allowed).to.equal(testcase.permitted)
}
})
})
describe('nameing conventions', () => {
let poller
beforeEach(() => {
poller = pollerFactory()
})
it('should restrict access as defined in namespace naming convention ', () => {
const testcases = [
{
// no annotations at all
ns: { metadata: {} },
descriptor: {},
permitted: true
},
{
// empty annotation
ns: { metadata: { annotations: { [namingPermittedAnnotation]: '' } } },
descriptor: {},
permitted: true
},
{
// test regex
ns: { metadata: { annotations: { [namingPermittedAnnotation]: '.*' } } },
descriptor: {
data: [
{ key: 'whatever', name: 'somethingelse' }
]
},
permitted: true
},
{
// test regex
ns: { metadata: { annotations: { [namingPermittedAnnotation]: 'dev/team-a/.*' } } },
descriptor: {
data: [
{ key: 'dev/team-a/secret', name: 'somethingelse' }
]
},
permitted: true
},
{
// test regex
ns: { metadata: { annotations: { [namingPermittedAnnotation]: 'dev/team-a/.*' } } },
descriptor: {
data: [
{ key: 'dev/team-b/secret', name: 'somethingelse' }
]
},
permitted: false
}
]

for (let i = 0; i < testcases.length; i++) {
const testcase = testcases[i]
const verdict = poller._isPermitted(testcase.ns, testcase.descriptor)
Expand Down

0 comments on commit c4fdea6

Please sign in to comment.