Repository navigation
Add release note and docs to credentials refactoring. #3299
Description
Activity
/help
@twoGiants:
This request has been marked as needing help from a contributor.Please ensure the request meets the requirements listed here.
If this request no longer meets these requirements, the label can be removed
by commenting with the/remove-helpcommand.Details
In response to this:
/help
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
/good-first-issue
@twoGiants:
This request has been marked as suitable for new contributors.Please ensure the request meets the requirements listed here.
If this request no longer meets these requirements, the label can be removed
by commenting with the/remove-good-first-issuecommand.Details
In response to this:
/good-first-issue
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
The flags is still hidden tho.
hey i would like to take up this issue
/assign @Kunal1522
Hey @Kunal1522 , Feel free to raise a PR.
you can create a small PR with some typo corrections for example, reference this issue and the mentioned PR and add a release note there with a link to another PR in the docs repo with the documentation update
hi @twoGiants i created the pr #knative/docs#6543 in docs repo and referenced this issue and #3298 there but i did not understand what
u meant byadd a release note there with a link to another PR in the docs repo
do i need to make another pr in docs repo and add the release notes and reference pr or i have to do something in this repo ?
I think few things need to be resolved first?
We have these values hidden, so running
func buildwon't show the flags.
We have this code which comments that it's only available for host builder so it needs to be updated and they should not be hidden anymore if that is the intentional behavior.
Also we should make sure there aren't any new test cases we are missinghi @gauron99 thanks for review
I ran a quick experiment - reverted the codebase to the commit right before PR #3298 and tested the s2i builder with FUNC_USERNAME/FUNC_PASSWORDThe push succeeded, which suggests the credential flow may have been working for other builders too - perhaps PR #3298 was more about consolidating/cleaning up the implementation rather than enabling new functionality
That said, I agree with your points if these flags are meant to work across all builders, we should unhide the flags, update the comments, and add test coverage for s2i/pack credential scenarios.
I'd be happy to take this on as a follow-up PR if that works!
would be great if @matejvasek could give his thoughts on this
I think we should make sure this works first before creating a release note/ updating docs. I would prefer we do this the other way around. tests,update comments, unhide flags etc. then update docs
I ran a quick experiment - reverted the codebase to the commit right before PR #3298 and tested the s2i builder with FUNC_USERNAME/FUNC_PASSWORD
thats interesting. Perhaps you are right. would be good to get to the bottom of this so we can clearly say if this works or not.
@Kunal1522 hey, can this be closed with #3394? is that complete?
also there is a docs PR from you knative/docs#6543.
Can we make that up-to-date and merge, if relevant still?hey @gauron99
the flags were still hidden so i have attached a pr which unhides them.
after merger of that pr we can update the docs ..
also one thing that i wanted to mention in docs was that using FUNC_PASSWORD env var is more secure than --password flag because command-line args are visible to all users via ps .so i will update that in docs as well.
after that i believe we can probably close this issueReacted by David Fridrich
Summary
After merging #3298 the following feature is now available:
FUNC_[USERNAME|PASSWORD]environment variables now work also for the pushers other that the host pusher.This needs to be added as a release note and into the official Knative func documentation.
You can create a small PR with some typo corrections for example, reference this issue and the mentioned PR and add a release note there with a link to another PR in the docs repo with the documentation update.
/kind documentation
/kind enhancement