external-secrets icon indicating copy to clipboard operation
external-secrets copied to clipboard

ExternalSecret pointing out to non-existent SecretStore key is shown as synced

Open magmax opened this issue 3 years ago • 6 comments

Describe the solution you'd like

Two scenarios:

  • Scenario A: A new ExternalSecret is created using a SecretStore key that doesn't exist. Currently, the secret is created without the key and the ExternalSecret shows a SecretSynced status.
  • Scenario B: to remove an existing key from a SecretStore what is still in use by an ExternalSecret. Currently, the key is removed from the secret and the ExternalSecret shows a SecretSynced status.

In both cases, it might cause any Pod mounting the secret to fail if restarted, so the ExternalSecret should show the real status like NotFoundRef, or other representative status, allowing to create alerts under that circumstance.

What is the added value?

ExternalSecret showing the real status when keys are not found.

Give us examples of the outcome

Files:

  • secretstore.yaml
apiVersion: external-secrets.io/v1beta1
kind: SecretStore
metadata:
  name: fake-secrets
spec:
  provider:
    fake:
      data:
      - key: "foo"
        value: "FOO"
        version: "v1"
      - key: "bar"
        value: "BAR"
        version: "v1"
  • secretstore2.yaml
apiVersion: external-secrets.io/v1beta1
kind: SecretStore
metadata:
  name: fake-secrets
spec:
  provider:
    fake:
      data:
      - key: "bar"
        value: "BAR"
        version: "v1"
  • externalsecret.yaml:
apiVersion: external-secrets.io/v1beta1
kind: ExternalSecret
metadata:
  name: example
spec:
  refreshInterval: 2m
  secretStoreRef:
    name: fake-secrets
    kind: SecretStore
  target:
    deletionPolicy: Delete
    name: secret
  data:
  - secretKey: foo
    remoteRef:
      key: foo
      version: v1
  - secretKey: bar
    remoteRef:
      key: bar
      version: v1
  • deploy.yaml:
apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx-deployment
  labels:
    app: nginx
spec:
  replicas: 1
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - name: nginx
        image: nginx:1.14.2
        env:
          - name: SECRET
            valueFrom:
              secretKeyRef:
                name: secret
                key: "foo"

Scenario A:

  • load files secretstore2.yaml and externalsecret.yaml, then deploy.yaml.
  • Secret will be created just with key bar and the pod will remain in CreateContainerConfigError status.

Scenario B:

  • load files secretstore.yaml and externalsecret.yaml, then deploy.yaml.
  • when pod becomes ready, load file secretstore2.yaml, what will remove the foo key from the secret in 2 mins.
  • kill the pod or try to scale it out. New pods will be in CreateContainerConfigError status because they do not find the secret.

Observations (Constraints, Context, etc):

Give here all extra information that could be interesting. Such as Golang version and Kubernetes version if you are reporting a bug/problem. You can also foresee technical constrains like "this could only be implementing using this specific technology or approach, because of this and that".

  • kubernetes 1.20
  • external-secrets operator 0.5.1, installed via helm chart 0.5.1

magmax avatar Apr 25 '22 10:04 magmax

Hi @magmax ! Can you check the Reason field value? I believe it should be set as SecretDeleted.

This is not an issue. This is by design.

The reasoning is that DeletionPolicy:Delete should treat not found errors as part of the lifecycle of the Secret.

specially if we have an ExternalSecret with multiple keys bound to it. IMO it wouldn’t make any sense to mark the whole object as in error due to the fact that one key is deleted from the provider.

It is up to the user to make sure the secret is available when using DeletionPolicy: Delete, as we cannot know if the secret has previously existed or not.

gusfcarvalho avatar Apr 25 '22 19:04 gusfcarvalho

Maybe you should try using DeletionPolicy: Retain (the default one) as it will keep the secret created, and thus allow your pods to keep starting (although the secret information will not necessarily be there in its newest version).

gusfcarvalho avatar Apr 25 '22 23:04 gusfcarvalho

Hi @magmax ! Did you test your issue with DeletionPolicy: Retain ?

gusfcarvalho avatar May 04 '22 09:05 gusfcarvalho

sorry, @gusfcarvalho , I will.

magmax avatar May 04 '22 09:05 magmax

Tested, @gusfcarvalho, and it worked: image

I think my problem was that I misunderstood documentation. Currently, it says:

Retain will retain the secret if all provider secrets have been deleted. If a provider secret does not exist the ExternalSecret gets into the SecretSyncedError status.

I expected "retain" to keep the secret even if the ExternalSecret is removed, what doesn't happen. I'd suggest to change it by:

Retain will retain the secret values if any of the required secrets is missing, moving the ExternalSecret into the SecretSyncedError status. In this case, none of the secrets will be updated.

Works for you? Maybe a third and even-better doc improvement?

magmax avatar May 04 '22 09:05 magmax

Your confusion is with what “provider” means. The ExternalSecrets manifest is not the provider, but rather the definition of a synchronization spec. You can have the behavior you mentioned with CreationPolicy: Orphan as this behavior is not related to what happens when the source of the secret (the provider) gets deleted (scope of DeletionPolicy).

Regarding your suggestion, what do you mean by “in this case, none of the secrets will be updated”?

gusfcarvalho avatar May 04 '22 21:05 gusfcarvalho

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar Dec 13 '22 02:12 github-actions[bot]

This issue was closed because it has been stalled for 30 days with no activity.

github-actions[bot] avatar Jan 13 '23 02:01 github-actions[bot]