Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for providing credentials when adding chart-repositories #67

Merged
merged 7 commits into from
Jan 7, 2019

Conversation

Flydiverny
Copy link
Contributor

@Flydiverny Flydiverny commented Dec 15, 2018

What this PR does / why we need it:

Add support for providing chart repo credentials when adding private chart repos

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:
First time writing go, So be nice <3

Copy link
Member

@unguiculus unguiculus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I agree this is something we need, but I think I'd rather go a different route here. We already have helm-extra-args for installing charts. We need something like this for helm repo as well. How about adding helm-repo-extra-args? This would be more consistent and not only allow specifying username and password but also TLS arguments.

@Flydiverny
Copy link
Contributor Author

Flydiverny commented Dec 16, 2018

Seems like a nicer solution! I pushed a variant with a helm-repo-extra-args config.
I did pretty much the same as the first solution to allow for passing different args for different repositories.

so you can do something like this

ct lint --chart-repos "private=https://private,ambassador=https://www.getambassador.io" \
        --helm-repo-extra-args "private=--username user --password secret" \
        --helm-repo-extra-args "ambassador=--no-update"

Or yaml config:

chart-repos:
  - incubator=https://incubator
  - private=https://private
helm-repo-extra-args:
  - private=--username test --password secret

pkg/chart/chart.go Show resolved Hide resolved
Copy link
Member

@unguiculus unguiculus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use ct doc-gen to generate the documentation. It seems you updated it manually.

app/cmd/root.go Outdated Show resolved Hide resolved
pkg/chart/chart.go Outdated Show resolved Hide resolved
app/cmd/root.go Outdated Show resolved Hide resolved

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…n adding chart repos

Signed-off-by: Markus Maga <[email protected]>
Signed-off-by: Markus Maga <[email protected]>
Signed-off-by: Markus Maga <[email protected]>
@Flydiverny
Copy link
Contributor Author

@unguiculus pr updated and rebased :)

README.md Outdated Show resolved Hide resolved
@unguiculus unguiculus merged commit 9084cf1 into helm:master Jan 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants