Repository: godaddy/kubernetes-external-secrets (js)

Issue:
Use of "iam.amazonaws.com/permitted" annotation causes conflicts with existing KIAM setups
## I'm submitting a...
* [X] bug report 

- related to `roleArn` functionality described in issue https://github.com/godaddy/kubernetes-external-secrets/issues/143
and implemented in PR https://github.com/godaddy/kubernetes-external-secrets/pull/144

- Use of KIAM `iam.amazonaws.com/permitted` annotation for `roleArn` validation causes conflicts with existing KIAM setups.

- `iam.amazonaws.com/permitted` is a KIAM created and managed annotation, and not part of any known standard.
(e.g. kube2iam uses a different annotation: `iam.amazonaws.com/allowed-roles` )

## Issue 1:

### What is the current behavior?

Kubernetes external secrets errors out when `iam.amazonaws.com/permitted` is present, has valid values intended for `kiam`, but is not provided a `roleArn` in the `ExternalSecret` descriptor.

This is a common use case when `kiam` is handling the IAM for the `kubernetes-external-secret` operator, and no additional role assumption is required by the individual `ExternalSecret` resources

To reproduce:

``` 
kind: ExternalSecret
metadata:
  annotations:
  name: test-service
secretDescriptor:
 #No Role ARN Provided
  backendType: secretsManager
  data:
  - key: test-service/password
    name: password
```

Namespace annotated for `kiam` support only:

```
kind: Namespace
metadata:
  annotations:
    iam.amazonaws.com/permitted: my-kiam-roles.* 
```

``` 
{"level":50,"time":XXX,"pid":16,"hostname":"XXX","type":"Error","stack":"Error: not allowed to fetch secret: test-service: namspace does not allow to assume role undefined
    at Poller._upsertKubernetesSecret (/app/lib/poller.js:110:14)
    at process._tickCallback (internal/process/next_tick.js:68:7)","msg":"failure while polling the secret test-service","v":1}
```

### What's the expected behavior?

Secret sync should be allowed when namespace annotation is present, and no `roleArn` is provided.

```         
{
     // test undefined
     ns: { metadata: { annotations: { 'iam.amazonaws.com/permitted': 'my-kiam-role.*' } } },
     descriptor: { roleArn: undefined },
     permitted: true
}, 
```

## Issue 2

### What is the current behavior?

`kiam` or `kubernetes-external-secrets` will now **incorrectly permit** (i.e. pass the regex and proceed to attempt an sts call) assumption of roles in a namespace not intended to be used by `kiam` or `kubernetes-external-secrets`.  The sharing of an annotation allows for nondeterministic behavior from each operator when used at the same time.  

Scenario (AWS ARNs truncated for brevity):

- `kubernetes external secret` controller uses `kiam` to assume roles for processing, with that role (and all other allowed `kiam` roles) covered by the `iam.amazonaws.com/permitted: kiam-roles.*` annotation on the namespace

- `kubernetes external secrets` `ExternalSecret` has a desired `roleArn` value `my-team-external-secret-role.*`

- `kiam` policies are attached to `kiam-server` pods, and have no knowledge of `ExternalSecret` level `kubernetes-external-secret` role / polices 

- `kubernetes-external-secret` policies are assumed at runtime from the `kubernetes-external-secret` controller AFTER it assumes a role via `kiam`, and have no knowledge of all the other `kiam` level role / polices 

In order to support this, one of the following must now happen:

- regex updated to support both patterns: `(kiam|my-team-external-secret-role).*`
  - now `kiam` OR `kubernetes-external-secrets` is _"permitting"_ roles they can not actually assume 
- naming convention updated to support a single shared pattern: `namespace-iam-roles.*` 
  - same result, `kiam` OR `kubernetes-external-secrets` is still _"permitting"_ roles they can not actually assume 

### What's the expected behavior?

One of the following should be done to `kubernetes-external-secrets` prevent conflicts
- Backwards Compatible Break - change the support annotation for  `kubernetes-external-secrets` to be something under the existing CRD naming of `externalsecrets.kubernetes-client.io`, 

- Allow the annotation to be configurable so that both `kiam` and `kubernetes-external-secrets` can run without conflict

- Explicitly document this issue in the `README.md` 


Pull request description:
fix: iam role
This PR fixes #174:

* (bug) sync should work when namespace annotation is present, and no roleArn is provided
* (feature) make the role-permission annotation key configurable

These issues are addressed in two separate commits. 

Patch:
diff --git a/README.md b/README.md
index a376997..d6dfb0b 100644
--- a/README.md
+++ b/README.md
@@ -55,6 +55,7 @@ The following table lists the configurable parameters of the `kubernetes-externa
 | `env.AWS_REGION`                          | Set AWS_REGION in Deployment Pod                             | `us-west-2`                                             |
 | `env.LOG_LEVEL`                           | Set the application log level                                | `info`                                                  |
 | `env.METRICS_PORT`                        | Specify the port for the prometheus metrics server           | `3001`                                                  |
+| `env.ROLE_PERMITTED_ANNOTATION`           | Specify the annotation key where to lookup the role arn permission boundaries | `iam.amazonaws.com/permitted`          |
 | `env.POLLER_INTERVAL_MILLISECONDS`        | Set POLLER_INTERVAL_MILLISECONDS in Deployment Pod           | `10000`                                                 |
 | `envVarsFromSecret.AWS_ACCESS_KEY_ID`     | Set AWS_ACCESS_KEY_ID (from a secret) in Deployment Pod      |                                                         |
 | `envVarsFromSecret.AWS_SECRET_ACCESS_KEY` | Set AWS_SECRET_ACCESS_KEY (from a secret) in Deployment Pod  |                                                         |
@@ -87,14 +88,14 @@ Set AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY env vars in the session/pod.
 You can use envVarsFromSecret in the helm chart to create these env vars from existing k8s secrets
 
 Additionally, you can specify a `roleArn` which will be assumed before retrieving the secret.
-You can limit the range of roles which can be assumed by this particular *namespace* by using annotations on the namespace resource.
-The annotation value is evaluated as a regular expression and tries to match the `roleArn`.
+You can limit the range of roles which can be assumed by this particular *namespace* by using annotations on the namespace resource. The annotation key is configurable (see above). The annotation value is evaluated as a regular expression and tries to match the `roleArn`.
 
 ```yaml
 kind: Namespace
 metadata:
   name: iam-example
   annotations:
+    # annotation key is configurable
     iam.amazonaws.com/permitted: "arn:aws:iam::123456789012:role/.*"
 ```
 
diff --git a/bin/daemon.js b/bin/daemon.js
index c155cf5..ec7986e 100755
--- a/bin/daemon.js
+++ b/bin/daemon.js
@@ -21,7 +21,8 @@ const {
   customResourceManifest,
   logger,
   metricsPort,
-  pollerIntervalMilliseconds
+  pollerIntervalMilliseconds,
+  rolePermittedAnnotation
 } = require('../config')
 
 async function main () {
@@ -46,6 +47,7 @@ async function main () {
     kubeClient,
     metrics,
     pollerIntervalMilliseconds,
+    rolePermittedAnnotation,
     customResourceManifest,
     logger
   })
diff --git a/charts/kubernetes-external-secrets/README.md b/charts/kubernetes-external-secrets/README.md
index 7afae8f..9be45f9 100644
--- a/charts/kubernetes-external-secrets/README.md
+++ b/charts/kubernetes-external-secrets/README.md
@@ -39,6 +39,7 @@ The following table lists the configurable parameters of the `kubernetes-externa
 | `env.AWS_REGION`                     | Set AWS_REGION in Deployment Pod                             | `us-west-2`                                             |
 | `env.LOG_LEVEL`                           | Set the application log level                                | `info`                                                  |
 | `env.METRICS_PORT`                        | Specify the port for the prometheus metrics server           | `3001`                                                  |
+| `env.ROLE_PERMITTED_ANNOTATION`           | Specify the annotation key where to lookup the role arn permission boundaries | `iam.amazonaws.com/permitted`          |
 | `env.POLLER_INTERVAL_MILLISECONDS`   | Set POLLER_INTERVAL_MILLISECONDS in Deployment Pod           | `10000`                                                 |
 | `envVarsFromSecret.AWS_ACCESS_KEY_ID`     | Set AWS_ACCESS_KEY_ID (from a secret) in Deployment Pod      |                                                         |
 | `envVarsFromSecret.AWS_SECRET_ACCESS_KEY` | Set AWS_SECRET_ACCESS_KEY (from a secret) in Deployment Pod  |                                                         |
diff --git a/config/environment.js b/config/environment.js
index ee7b0a8..94a221f 100644
--- a/config/environment.js
+++ b/config/environment.js
@@ -21,11 +21,14 @@ const pollerIntervalMilliseconds = process.env.POLLER_INTERVAL_MILLISECONDS
 
 const logLevel = process.env.LOG_LEVEL || 'info'
 
+const rolePermittedAnnotation = process.env.ROLE_PERMITTED_ANNOTATION || 'iam.amazonaws.com/permitted'
+
 const metricsPort = process.env.METRICS_PORT || 3001
 
 module.exports = {
   environment,
   pollerIntervalMilliseconds,
   metricsPort,
+  rolePermittedAnnotation,
   logLevel
 }
diff --git a/lib/poller-factory.js b/lib/poller-factory.js
index 21627c2..d683a8f 100644
--- a/lib/poller-factory.js
+++ b/lib/poller-factory.js
@@ -11,12 +11,14 @@ class PollerFactory {
    * @param {Object} customResourceManifest - CRD manifest
    * @param {Object} logger - Logger for logging stuff.
    * @param {number} pollerIntervalMilliseconds - Interval time in milliseconds for polling secret properties.
+   * @param {String} rolePermittedAnnotation - namespace annotation that defines which roles can be assumed within this namespace
    */
   constructor ({
     backends,
     kubeClient,
     metrics,
     pollerIntervalMilliseconds,
+    rolePermittedAnnotation,
     customResourceManifest,
     logger
   }) {
@@ -26,6 +28,7 @@ class PollerFactory {
     this._kubeClient = kubeClient
     this._pollerIntervalMilliseconds = pollerIntervalMilliseconds
     this._customResourceManifest = customResourceManifest
+    this._rolePermittedAnnotation = rolePermittedAnnotation
   }
 
   /**
@@ -40,6 +43,7 @@ class PollerFactory {
       logger: this._logger,
       metrics: this._metrics,
       customResourceManifest: this._customResourceManifest,
+      rolePermittedAnnotation: this._rolePermittedAnnotation,
       externalSecret
     })
 
diff --git a/lib/poller.js b/lib/poller.js
index d19523b..333b293 100644
--- a/lib/poller.js
+++ b/lib/poller.js
@@ -12,8 +12,6 @@
  *   object, this is the property name of the value to use.
  */
 
-const annotationPermittedKey = 'iam.amazonaws.com/permitted'
-
 /** Poller class. */
 class Poller {
   /**
@@ -25,6 +23,7 @@ class Poller {
    * @param {string} namespace - Kubernetes namespace.
    * @param {Object} customResourceManifest - CRD manifest
    * @param {Object} externalSecret - ExternalSecret manifest.
+   * @param {string} rolePermittedAnnotation - namespace annotation that defines which roles can be assumed within this namespace
    * @param {Object} metrics - Metrics client.
    */
   constructor ({
@@ -34,6 +33,7 @@ class Poller {
     logger,
     metrics,
     customResourceManifest,
+    rolePermittedAnnotation,
     externalSecret
   }) {
     this._backends = backends
@@ -42,6 +42,7 @@ class Poller {
     this._logger = logger
     this._timeoutId = null
     this._metrics = metrics
+    this._rolePermittedAnnotation = rolePermittedAnnotation
     this._customResourceManifest = customResourceManifest
 
     this._externalSecret = externalSecret
@@ -169,13 +170,13 @@ class Poller {
     let allowed = true
     let reason = ''
 
-    if (!namespace.metadata.annotations) {
+    if (!namespace.metadata.annotations || !role) {
       return {
         allowed, reason
       }
     }
     // an empty annotation value allows access to all roles
-    const re = new RegExp(namespace.metadata.annotations[annotationPermittedKey])
+    const re = new RegExp(namespace.metadata.annotations[this._rolePermittedAnnotation])
 
     if (!re.test(role)) {
       allowed = false


Test patch:
diff --git a/lib/poller.test.js b/lib/poller.test.js
index e6f37ad..4c30a2a 100644
--- a/lib/poller.test.js
+++ b/lib/poller.test.js
@@ -25,6 +25,8 @@ describe('Poller', () => {
     uid: fakeExternalSecret.metadata.uid
   })
 
+  const rolePermittedAnnotation = 'iam.amazonaws.com/permitted'
+
   beforeEach(() => {
     backendMock = sinon.mock()
     kubeClientMock = sinon.mock()
@@ -90,6 +92,7 @@ describe('Poller', () => {
         kubeClient: kubeClientMock,
         logger: loggerMock,
         externalSecret: fakeExternalSecret,
+        rolePermittedAnnotation,
         customResourceManifest: fakeCustomResourceManifest
       })
     }
@@ -440,7 +443,7 @@ describe('Poller', () => {
     })
 
     it('does not permit update of secret', async () => {
-      fakeNamespace.body.metadata.annotations['iam.amazonaws.com/permitted'] = '^$'
+      fakeNamespace.body.metadata.annotations[rolePermittedAnnotation] = '^$'
       poller = pollerFactory({
         backendType: 'fakeBackendType',
         name: 'fakeSecretName',
@@ -536,27 +539,33 @@ describe('Poller', () => {
         },
         {
           // empty annotation
-          ns: { metadata: { annotations: { 'iam.amazonaws.com/permitted': '' } } },
+          ns: { metadata: { annotations: { [rolePermittedAnnotation]: '' } } },
           descriptor: {},
           permitted: true
         },
         {
           // test regex
-          ns: { metadata: { annotations: { 'iam.amazonaws.com/permitted': '.*' } } },
+          ns: { metadata: { annotations: { [rolePermittedAnnotation]: '.*' } } },
           descriptor: { roleArn: 'whatever' },
           permitted: true
         },
         {
           // test regex: deny access
-          ns: { metadata: { annotations: { 'iam.amazonaws.com/permitted': '^$' } } },
+          ns: { metadata: { annotations: { [rolePermittedAnnotation]: '^$' } } },
           descriptor: { roleArn: 'whatever' },
           permitted: false
         },
         {
           // real world example
-          ns: { metadata: { annotations: { 'iam.amazonaws.com/permitted': 'arn:aws:iam::123456789012:role/.*' } } },
+          ns: { metadata: { annotations: { [rolePermittedAnnotation]: 'arn:aws:iam::123456789012:role/.*' } } },
           descriptor: { roleArn: 'arn:aws:iam::123456789012:role/somerole' },
           permitted: true
+        },
+        {
+          // test undefined
+          ns: { metadata: { annotations: { [rolePermittedAnnotation]: 'my-kiam-role.*' } } },
+          descriptor: {},
+          permitted: true
         }
       ]