vidispine / hull

The incredible HULL - Helm Uniform Layer Library - is a Helm library chart to improve Helm chart based workflows
https://github.com/vidispine/hull
Apache License 2.0
213 stars 12 forks source link

Creating non-opaque Secrets #274

Closed khmarochos closed 7 months ago

khmarochos commented 8 months ago

Hello again!

What do you think about allowing the user to set the type of a Secret? It's always Opaque, but a user might need to set another type (e.g., kubernetes.io/dockerconfigjson).

I can try to make a PR if it's appropriate. :-)

gre9ory commented 8 months ago

I had found couple of times that it would be appropriate to be able to create other secrets such as token secrets as well. Then I always managed to dodge the scenarios that would require implementing it so skipped it. For the docker configs of course you can use the registry object type which I would recommend since the set of features around it should simplify image handling in general.

But yeah, of course I would welcome your PR for full configurability.

From https://kubernetes.io/docs/concepts/configuration/secret/ I get that the type may even be freely defined (didn't know that yet :) ) so it should just be possible to set any string value for the type. Implementationwise it should be sufficient to remove the hard-coding of type: Opaque. Default if not provided should still be type: Opaque of course. The JSON schema for io.k8s.api.core.v1.Secret already allows to specify the type so no changes to the schema should be required - actually a bug since the hard-coding and an additional free specification of kind would have caused problems likely. There are some additional validations applied to some types the Kubernetes docs say but I don't feel it is something that has an impact on a first implementation now. These are just my thoughts on implementation, feel free to provide yours.

For the PR it would be good if you add your code to the fixes-1.29 branch and open a PR to merge to release-1.29. If everything is looking good we can do the same for fixes-1.27 and fixes-1.28 and also merge fixes-1.29 to main last.

Thank you!

khmarochos commented 8 months ago

Hello!

I'm not sure that my solution is correct. Perhaps I just don't know some architectural aspects of HULL, so my patch might turn out to be too straightforward.

And I'm very sorry for the newline character in the end of the template file. I didn't notice it before to make a PR. :-(

gre9ory commented 8 months ago

@khmarochos @SquupS

Starting from releases:

1.29.2 1.28.8 1.27.10

it is now possible to set the type of secrets.

Thank you!

khmarochos commented 8 months ago

Hooray! Thank you for the update!

gre9ory commented 7 months ago

@khmarochos Closing this issue hoping the solution provided works for you. Otherwise feel free to restart the discussion here.

Thank you for the participation!

gre9ory commented 7 months ago

Closing