feat: add templating to PushSecret

[Skarlso]

Problem Statement

Add templating to Push Secret. This is a highly in-progress pull request with lots of things missing. But shows progress on the issue.

Also, I want to avoid merge conflicts.

Todos

• properly understand the outcome
• lots and lots of unit tests

Related Issue

Fixes #2166

Proposed Changes

How do you like to solve the issue and why?

Checklist

• I have read the contribution guidelines
• All commits are signed with git commit --signoff
• My changes have reasonable test coverage
• All tests pass with make test
• I ensured my PR is ready for review with make reviewable

[Skarlso]

@moolen How bad is this? :D Either this, or I have to move out the templating into another package. That causes problems with the deepEqual methods. Or, I move pushsecret to v2, but we don't just wanted that here right now. If we don't do that, we will have to settle with the duplication from the previous commit. :(

[moolen]

As long as it compiles, it is fine 😆 If an alpha API contains a beta API that makes it more stable, right? :D
I think this should be fine, it looks odd, but that's about it.

[Skarlso]

Bwuhahahaha. Okay, thanks. :D

[Skarlso]

These duplications will be a bit of a pain to get rid of considering they are only different because they work on different types.

[knelasevero]

These duplications will be a bit of a pain to get rid of considering they are only different because they work on different types.

I see you already addressed some of the duplication. I agree it would not be trivial to get rid of the duplication still present there. I think that can also come as an enhancement in a follow up PR.

Regarding going to v1alpha2 or beta, that can also happen soon, and would help a bit as you pointed out. We might want to change these versions at the next minor version bump of the project?

[knelasevero]

Wanted to see some e2e tests here. That would help see a few things that might be missing from the implementation and edge cases

[Skarlso]

Thanks @knelasevero ! Yes, to all of your questions. :D The duplication I mostly got down. But the rest will be super tricky. :/ They do the same thing, yes, but without some forced interfacing it would be impossible to get rid of. Since it's working with different types.

I agree with the e2e part. I'll try to whip something up. :)

[Skarlso]

Hah, actually spotted a lot more duplication that I could throw out. :)

[Skarlso]

/ok-to-test sha=76ed5d205daca8fa6e0dad3101e173b159047b13

[Skarlso]

@knelasevero programatically, or done by hand? (the e2e tests).

[knelasevero]

@knelasevero programatically, or done by hand? (the e2e tests).

programatically

[Skarlso]

@knelasevero Ok, I'll try. :D

[Skarlso]

Okay, so basic case is working manually

kubectl create secret generic git-sync-secret --from-literal=username=USER --from-literal=password=TOKEN--from-literal=token=TOKEN

SecretStore:

apiVersion: external-secrets.io/v1beta1
kind: SecretStore
metadata:
  name: secret-store-name
  namespace: default
spec:
  provider:
    kubernetes:
      auth:
        serviceAccount:
          # This would be the project service account which has the right permissions to create secrets.
          name: "default"
      server:
        caProvider: # we are connecting to our own cluster.
          type: ConfigMap
          name: kube-root-ca.crt
          key: ca.crt

PushSecret:

➜  push-secrets-templating cat push_secret_with_template.yaml
apiVersion: external-secrets.io/v1alpha1
kind: PushSecret
metadata:
  name: example-push-secret-with-template
spec:
  refreshInterval: 10s
  secretStoreRefs:
    - name: secret-store-name
      kind: SecretStore
  selector:
    secret:
      name: git-sync-secret
  template:
    engineVersion: v2
    data:
      token: "{{ .token | toString | upper }} was templated"
  data:
    - match:
        secretKey: token
        remoteRef:
          remoteKey: git-sync-secret-copy-templated
          property: token

And then describing the secret

token: TOKEN was templated

That's nice. I'll try to write some e2e tests for this.

[Skarlso]

@knelasevero I tried adding a Test case. But there is no PushSecret tests in there. The base test case requires some external secrets to be set up and it makes the push secret test case just so convoluted since it has to set up external secrets.

I would have to refactor the way the whole default test case setup works and honestly, I don't want to do that in this PR.

I'm super happy to refactor the entire thing in a follow up after this. This is an optional feature after all.

The reason for it is that I really really hate prs that span over a 1000 lines and I'm coming really close to that as is. :)

What do you folks think?

Edit: To be more precise, there are no facilities for PushSecret testing at all. No store that can handle the creation, no checks for the end result, nothing. :D

[moolen]

Thanks for implementing this, this looks really nice, i like it! ❤️
I did manual testing as well, works like a charm 👯

Added some comments below, i think we can simplify the PushSecret template handling a bit, otherwise LGTM and 🚀

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.8% Duplication on New Code

See analysis details on SonarCloud

[moolen]

/ok-to-test sha=965c9da

[moolen]

LGTM, really really great feature, thank you! 🙇

[Skarlso]

@moolen Thank you for all the help 🙇 I shall begin writing the e2e tests and refactor and add PushSecrets. :) Thank you for testing it. I truly appreciate it!

[mruoss]

Ooooh this is awesome! I've had to write a cronjob to change formats! Now it's just a template within the PushSecret. 🙏

[Skarlso]

Yaaaay! :) Super happy that it helped. :))