tuenti / secrets-manager

A daemon to sync Vault secrets to Kubernetes secrets
Apache License 2.0
171 stars 26 forks source link

Feature/k8s labels annotations #64

Closed stevendborrelli closed 3 years ago

stevendborrelli commented 4 years ago

This adds functionality to copy Labels and Annotations from the SecretDef to the generated secret. This is needed for tools like Tekton Git authentication, and addresses #62

This also updates the managed-by and updatedAt labels to more closely match k8s recommended values (using annotations and recommended labels), as seen below:

annotations:
    secrets-manager.tuenti.io/lastUpdateTime: 2020-04-22T14.34.17Z
labels:
   app.kubernetes.io/managed-by: secrets-manager

If the Security definitions have the following labels & annotations:

metadata:
  labels:
    testlabel.example.com: "true"
    andthisonetoo: "true"
  annotations:
    tekton.dev/git-0: https://github.com

Then the secret generated will have the following labels & annotations:

kind: Secret
metadata:
  annotations:
    secrets-manager.tuenti.io/lastUpdateTime: 2020-04-22T14.34.17Z
    tekton.dev/git-0: https://github.com
  creationTimestamp: "2020-04-22T14:34:17Z"
  labels:
    andthisonetoo: "true"
    app.kubernetes.io/managed-by: secrets-manager
    testlabel.example.com: "true"
raelga commented 4 years ago

+1 to this PR.

We are using SecretDefinitions for ArgoCD cluster secrets. Those secrets are filtered by the argo server by using argocd.argoproj.io/secret-type: cluster label.

raelga commented 4 years ago

@eduardogr @fcgravalos do you need any help maintaining this project? We are using it internally and we would love to help keeping the project moving forward.

eduardogr commented 4 years ago

@eduardogr @fcgravalos do you need any help maintaining this project? We are using internally and we would love to help keeping the project moving forward.

@raelga i'm going to review this PR this week. And i'll talk about your proposal with my teammates. Do you want to talk about your usecases in a 1:1 chat or something?

eduardogr commented 4 years ago

LGTM :+1: Thanks for this PR @stevendborrelli and sorry for our delay reviewing it.

Can you please resolve conflicts within the file controllers/secretdefinition_controller_test.go?? After that i'll merge this PR

:pray: Thanks a lot!

stevendborrelli commented 4 years ago

Yes, I'll take a look at the conflicts. Thanks for the review!

eduardogr commented 3 years ago

Yes, I'll take a look at the conflicts. Thanks for the review!

@stevendborrelli do you need some help with this?

raelga commented 3 years ago

Hello @stevendborrelli, do you need any help with this PR?

eduardogr commented 3 years ago

@stevendborrelli do you want us to continue this branch in order to integrate this feature? Could we help you ??

stevendborrelli commented 3 years ago

@eduardogr if you want to take a look at it. There is a merge conflict that brings up some other code issues that I am not familiar with. I can review it again so see if we can rebase it against the newest commits in master.

eduardogr commented 3 years ago

@eduardogr if you want to take a look at it. There is a merge conflict that brings up some other code issues that I am not familiar with. I can review it again so see if we can rebase it against the newest commits in master.

@stevendborrelli I will and I will contact you as soon as I have

eduardogr commented 3 years ago

@stevendborrelli what do you think if i create a branch from this one to solve conflicts and merge it in a different PR?

eduardogr commented 3 years ago

I've just create the PR #78 from this branch and adding more changes. If you @stevendborrelli are OK with this, i'll proceed with the integration of your changes.

Thanks in advance

stevendborrelli commented 3 years ago

Sounds good to me! I'll take a look at PR #78 and close this PR.

eduardogr commented 3 years ago

Thanks a lot @stevendborrelli