Include envFrom in "how to use a Secret as an environment source"
#8620
Conversation
I suggest that using envFrom for well known secrets types is recommended (or at least documented as an option), https://kubernetes.io/docs/concepts/configuration/secret/#basic-authentication-secret
tekton-robot
added
the
do-not-merge/release-note-label-needed
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
size/S
|
/kind documentation |
tekton-robot
added
kind/documentation
There was a problem hiding this comment.
Thanks for the PR @jcjveraa. I've added a couple of comments on the specific changes below, but also have some more general thoughts / questions.
The existing example is an actual functional task demonstrating use of the env approach to reading a value from a Secret. Would it make sense to leave that as-is, and instead add a second example showing the envFrom approach?
I'd be interested in others' thoughts on this. I think we should aim to keep the examples relatively simple yet realistic to clearly demonstrate the topic / feature, and avoid combining too many different pieces in a single example.
docs/tasks.md
Outdated
| envFrom: | ||
| - secretRef: | ||
| name: $(params.my-basic-auth-ssh-or-docker-type-secret-name} | ||
| # Using configMapRef works, but as al ConfigMaps are freeform this is not recommended |
There was a problem hiding this comment.
I'm not sure this distinction is particularly useful or something Tekton should take a stance on. Secrets are also freeform, although there are some well-known / agreed shapes commonly used.
In either case, regardless of using a ConfigMap or a Secret, it would be up to the Task author to ensure the expected shape / content is documented.
There was a problem hiding this comment.
In our organisation we have some less experienced users that also (need to) make custom Tasks. I have noticed that giving 'complete but opinionated' examples tends to work best. In this particular case:
- use the short
envFromsyntax for well known secret types (and of course in the task-script itself then check if variables are set properly) - let the user know that configMapRef can also be done, nothing preventing it, but it might not be what you want as it makes it unclear what is injected as environmental variables exactly (you could get conflicts).
What do you prefer: remove the opinion, or inlcude it but spell it out more clearly in the text above the code example (which as you suggested would be a seperate header)? If the docs allow for a "tip-callout" markup, perhaps the "be careful with using envFrom with freeform Secrets/ConfigMaps as you might get more variables (from the Task user) than you expect" could be useful in such a tip-section?
docs/tasks.md
Outdated
| env: | ||
| - name: GOPATH | ||
| value: /workspace | ||
| - name: GITHUB_TOKEN | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: $(params.github-token-secret) | ||
| key: bot-token | ||
| key: $(params.github-token-secret-key) |
There was a problem hiding this comment.
This will not work as the param github-token-secret-key is not declared in spec.params. This applies to the other new params introduced above.
The examples should pass validation, and ideally actually be functional, so that users have a working base to start from.
Is making the key configurable necessary for the purposes of this example?
There was a problem hiding this comment.
Apologies, you are right of course. This was the result of 'oh this would actually be better' just before I logged off and went to bed.
As to 'why parameterize' (if I had done it correctly): again I see users 'unneccesarily hardcoding things' based off of copying examples. Part of the 'power' of using Tekton vs just making a bash script that does it all lies in such as the parameterization of task inputs, so I'm used to 'fully parameterize' (with sane defaults) any example I write.
What I wanted to do/should have done before submitting the PR was add parameters above such as:
- name: github-token-secret-key
type: string
default: bot-tokenBased on your comment, perhaps it would be better to keep it simple here and 'full parameterization with defaults' would be better placed in the 'actual code' example (which could be linked to from the text) as requested by @waveywaves ?
There was a problem hiding this comment.
That sounds like a reasonable approach 👍
|
@AlanGreene Thanks for the feedback. I'll reply to your comments above, I expect to implement some changes reflecting your comments by this evening (including splitting it off to a different header to avoid combining topics). note: if those changes are 'close to acceptable', I'm perfectly happy that my PR is edited by any of the maintainers to ensure consistent style. |
…vFrom example in tasks.md
tekton-robot
added
size/M
|
I'm fairly sure that the header-level for the section we're changing in this PR was wrong (see screenshot - the 'subheaders' have nothing to do with the first header), so I fixed these too now. I've tested the minimal example yaml I've made on request by @waveywaves, see second screenshot.
|
I suggest that using envFrom for well known secrets types is recommended (or at least documented as an option),
https://kubernetes.io/docs/concepts/configuration/secret/#basic-authentication-secret
Changes
I suggest that using envFrom for well known secrets types is recommended (or at least documented as an option) as it makes use of the existing 'typing' in kubernetes, and many times in practice secrets will be some form of (well known) authentication secret like https://kubernetes.io/docs/concepts/configuration/secret/#basic-authentication-secret.
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes