external-secrets / external-secrets

External Secrets Operator reads information from a third-party service like AWS Secrets Manager and automatically injects the values as Kubernetes Secrets.
https://external-secrets.io/main
Apache License 2.0
4.13k stars 755 forks source link

Implement GetAllSecrets for the Generic Webhook provider #2896

Open 6ixfalls opened 7 months ago

6ixfalls commented 7 months ago

Is your feature request related to a problem? Please describe. It's a negative user experience to use the Generic Webhook provider (and external-secrets as a whole) only to retrieve a single value, and force us to rely on other implementations for anything that requires GetAllSecrets rather than every key hardcoded in a resource.

Describe the solution you'd like The CRD could be redesigned to implement a boolean property, such as "forAllSecrets", which can be used to determine if a SecretStore's webhook configuration is for the GetSecret or for the GetAllSecrets. An alternative method of implementing this is adding a sub-provider, such as bulk-webhook, which could also be used (with the same configuration as webhook) to have a secret store to support that feature. However, this may require more work for new features in the webhook provider, as any changes need to be replicated in both. Finally, another alternative is that the webhook provider can be reworked to move the current spec or configuration into another object, and could be structured as such:

apiVersion: external-secrets.io/v1beta1
kind: ClusterSecretStore
metadata:
  name: cluster-secret-store
spec:
  provider:
    webhook:
      single:
        url: "http://example.com/{{ .remoteRef.property }}"
        method: GET
        timeout: 5s
        result:
          jsonPath: "$.secret.secretValue"
      bulk:
        url: "http://example.com/all"
        method: GET
        timeout: 5s
        result:
          jsonPath: "$.secrets"

In theory, it may also be possible to simply add another URL and result for GetAllSecrets, but I'm not sure if that would be as useful. For my use-case and the provider I use, Infisical, all of these implementations work. In addition, I don't have any experience with jsonPath, and I'm not sure how you would handle an array of keys and values, and that would have to be discussed further. This is a rough draft of my ideas on how this could be implemented, and any ideas or suggestions would be great.

Describe alternatives you've considered There is no alternate feature. I use external-secrets as a "one-stop shop" for secret management in my Kubernetes cluster, and without this feature, I have to use a custom secret operator that supports my provider. However, that operator is lacking features as well, and as a result, I use external-secrets alongside that operator to have all the features I need.

Additional context https://github.com/external-secrets/external-secrets/issues/1110 https://github.com/external-secrets/external-secrets/issues/1110#issuecomment-1815589098

moolen commented 7 months ago

Are you trying to integrate this API: https://infisical.com/docs/api-reference/endpoints/secrets/list ?

GetAllSecrets() is not supported, that's right but GetSecretMap() is, which also allows you to return multiple key/value pairs.

To better understand what you're trying to do: how would the bulk work (in terms of what HTTP requests would be sent off)? Can you guide me through a flow? I think - maybe - it is already possible to fetch multiple secrets from an API.

As for the design: we need compatibility with existing versions, we can't just split the provider into two sub providers single and bulk.

6ixfalls commented 7 months ago

That's correct, I'm trying to implement that listing endpoint. While GetSecretMap is supported, it's a negative experience, since in Infisical, I'd have to either maintain the secrets inside a single key or have a key with references to other secrets, which would have to be maintained when adding or removing different secrets inside the map. In this case, as well as the case for most providers, a dedicated endpoint can be used to list secrets, and that endpoint's response would need to be parsed. The response for Infisical's raw endpoint returns an array inside a secrets key, which is an array of objects similar to the schema of the individual secret retrieving. There should be a way to parse this array of objects, use the key from one of the values, and use the value from another, in this case, secretKey and secretValue.

moolen commented 7 months ago

I think you should be able to do this today :thinking: While figuring this out i found a couple of edge cases with the webhook provider, e.g.: #2898.

What you can do is the following: you fetch the data from the /list endpoint and treat the response as a string (don't specify a jsonPath) and use the template functionality to render a dynamic json out of it.

That way you don't need to know the keys in advance.

apiVersion: external-secrets.io/v1beta1
kind: SecretStore
metadata:
  name: webhook
spec:
  provider:
    webhook:
      url: "https://httpbin.org/anything"
      result: {}
      headers:
        Content-Type: application/json
      body: |
        {
          "secrets": [
            {
              "_id": "abc123",
              "environment": "dev",
              "secretComment": "Lorem ipsum",
              "secretKey": "STRIPE_KEY",
              "secretValue": "abc123",
              "version": 1,
              "workspace": "abc123"
            },
            {
              "_id": "xxx666",
              "environment": "not-dev",
              "secretComment": "foobar",
              "secretKey": "OTHER_KEY",
              "secretValue": "xxx666",
              "version": 42,
              "workspace": "abc123"
            }
          ]
        }

---
apiVersion: external-secrets.io/v1beta1
kind: ExternalSecret
metadata:
  name: webhook
spec:
  data:
  - remoteRef:
      key: example
    secretKey: example
  refreshInterval: 1h
  secretStoreRef:
    kind: SecretStore
    name: webhook
  target:
    name: test-secret
    template:
      templateFrom:
        - target: Data
          literal: |
            {{- $payload := .example | fromJson }}
            {{- range $payload.json.secrets }} 
            {{ .secretKey }}: '{{ .secretValue }}'
            {{- end }}
# note: you will have to change
# {{- range $payload.json.secrets }} 
# to
# {{- range $payload.secrets }} 

This yields a secret with the following key/value pairs:

OTHER_KEY: xxx666
STRIPE_KEY: abc123

Though i must say that you're use-case is still valid. If i understood you correctly, the purpose of a GetAllSecrets() implementation would/could work like this:

  1. ESO makes a request to some API, let's call it /list which returns a list of names ["foo", "bar", "baz", ...]
  2. ESO then iterates over that list and for each entry it executes a HTTP request, let's say to /get/{ item }, /get/foo, /get/bar, ..

That would result in a secret:

foo: {value-of /get/foo response}
bar: {value-of /get/bar response}
...
6ixfalls commented 7 months ago

That sounds interesting, I'll try that out and see if it works. However, there will need to be manual refactoring for that, such as regexes which won't work with this, and secrets have to manually be changed while other providers don't require this. I'm not sure if sending a request for every value is a good idea, I'd expect a list endpoint to include the values as well because, with 100 different keys, you'd have to send 101 different requests to get all the values.

SimonStiil commented 2 months ago

By coincidence I stumbled on this thread.

I have been working on a REST based Key Value Database to specifically support External Secrets Webhook use-cases, if one wanted to work with something a lot simpler then the cloud providers. I was looking into also supporting PushSecrets but found it not implemented and found this issue as well.

I made my REST service to be as close to the regular API spec for fetching, updating, listing and deleting values and following those specs I think it could probably be implemented without changing the provider spec.

It would limit the current flexibility of the Generic Webhook provider as it currently support both setting body and method.

/keys/ used as an example but could just be www.example.com/ Method Endpoint Result
GET /keys/ List all keys
GET /keys/{{ .remoteRef.key }} Get specific key
PUT /keys/{{ .remoteRef.key }} Set a specific key to a value
POST /keys/{{ .remoteRef.key }} Set a specific key to a value
Delete /keys/{{ .remoteRef.key }} Delets the key
6ixfalls commented 2 months ago

Revisiting this as well because I'm running into the issue where PushSecrets just don't work with webhook providers. The logic for that should be relatively simple, just that the inverse of the GET webhook needs to be implemented (POST or PUT), like what was mentioned above.