search

carvel-dev / secretgen-controller

secretgen-controller provides CRDs to specify what secrets need to be on Kubernetes cluster (to be generated or not)
Apache License 2.0
177 stars 29 forks source link

Support for the Provisioned Service defined in the Service Binding Specification for Kubernetes #95

Open baijum opened 2 years ago

baijum commented 2 years ago

Describe the problem/challenge you have

I want to use the SecretTemplate API with implementations of the Service Binding Specification for Kubernetes.

Describe the solution you'd like

Make the SecretTemplate API a Provisioned Service as defined in the Service Binding Specification for Kubernetes.

Anything else you would like to add:

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you would like to work on this issue.

joe-kimmel-vmw commented 2 years ago

Thanks @baijum for contributing. I'm not familiar with the docs you linked but from a quick read it seems like you're proposing that we conform to this standard or API. Can you tell us more about your use-case, what your goals, and how they would be made simpler or enabled by SecretTemplate adopting these standards?

Samze commented 2 years ago

For reference we had a bit of discussion about this in the original issue see https://github.com/vmware-tanzu/carvel-secretgen-controller/issues/54#issuecomment-1117046143

In the end we decided to not adopt it by default initially for two reasons:

  1. Not all uses of SecretTemplate generate a secret that is logically "bindable".
  2. The k8s-binding specification allows you to refer to Secrets directly, which gives us an avenue to a basic integration with the binding specification without implementing the ProvisionedService duck type.

However we left the door open for this being added in the future.

I think providing compliance to ProvisionedService could make sense, but if we did it we would need to make it optional as part of the spec of the template. Something like .spec.provisionedService: true.

@baijum what are your thoughts on using a direct secret reference vs ProvisionedService?

baijum commented 2 years ago

Thanks, @Samze, for pointing out the original issue (#54) with a discussion about Provisioned Service.

@joe-kimmel-vmw As described in the spec, Provisioned Service expects .status.binding.name attribute pointing to a Secret resource. I have written a blog post about it.

My current use case is to use AWS services using the AWS Controllers for Kubernetes and SecretTemplate. Here is an example written by @Samze : https://github.com/Samze/secrettemplate-examples/tree/main/02-aws-rds If SecretTemplate conforms to the ProvisionedService, I could use it for binding using the ServiceBinding resource as per the spec.

Regarding the use of Direct Secret Reference, there is a good explanation in the spec docs:

Note: Implementing the Provisioned Service contract frees the ServiceBinding creator from needing to know about the name of the Secret holding the credentials. The service can update the secret name exposed over time. This behavior not only decouples the ServiceBinding and the workload from needing to know the name of the Secret, it also enables use of immutable secrets. An immutable secret cannot be updated after it is created. To rotate the credentials, a new secret can be created and update the same in the Provisioned Service resource’s .status.binding.name attribute.

If SecretTemplate introduces a boolean flag, .spec.provisionedService, to conform to Provisioned Service, that would be a great solution.

Samze commented 2 years ago

Thanks @baijum!

I agree this would be a good change to include. If theres broad agreement from other folks, would you be interested in contributing it?

Though just to be clear we already integrate using a direct secret reference today. This works since today the generated bindable Secret is always the same name as the SecretTemplate itself. So by knowing the SecretTemplate name, you can create a ServiceBinding with a direct reference to the generated Secret.

baijum commented 2 years ago

I agree this would be a good change to include. If theres broad agreement from other folks, would you be interested in contributing it?

I am glad to hear that there is an interest in supporting this feature. However, I will wait for a consensus before proceeding with implementation.

Samze commented 2 years ago

@cppforlife / @joe-kimmel-vmw / @neil-hickey do you have thoughts on this?

joe-kimmel-vmw commented 2 years ago

Thanks @Samze and @baijum IIRC we had a very short chat about it last week and thought it sounded fine, esp. if someone wanted to contribute it. I'll let @cppforlife or @neil-hickey correct if my recollection is way off.

cppforlife commented 2 years ago

it's good by me (sorry, i thought i already posted a comment)

baijum commented 2 years ago

I think providing compliance to ProvisionedService could make sense, but if we did it we would need to make it optional as part of the spec of the template. Something like .spec.provisionedService: true.

Does introducing an optional boolean field with a default value as false requires an API version upgrade?

cppforlife commented 2 years ago

Does introducing an optional boolean field with a default value as false requires an API version upgrade?

nope, we are happy to accept it as part of the current api version.