Skip to content

Comments

feat: adding ability for for charts to be pulled with plain HTTP#1672

Merged
yxxhero merged 9 commits intohelmfile:mainfrom
ennekein:main
Aug 27, 2024
Merged

feat: adding ability for for charts to be pulled with plain HTTP#1672
yxxhero merged 9 commits intohelmfile:mainfrom
ennekein:main

Conversation

@ennekein
Copy link
Contributor

accomplished by:

  • Adding PlainHttp attribute to RepositorySpec, HelmDefault, ReleaseSpec
  • When set to true, pass --plain-http to helm commands

Tweaked #1228 by @hoangelos

Resolves #1224

@ennekein
Copy link
Contributor Author

@yxxhero I see the test stage has failed, but I do not see any failed test. Any idea what the issue could be?

@yxxhero
Copy link
Member

yxxhero commented Aug 19, 2024

@ennekein oh. you can find the failed case by searching fail:. ctrl+F

@yxxhero
Copy link
Member

yxxhero commented Aug 19, 2024

@ennekein sorry. update the search keyword fail:.

@ennekein
Copy link
Contributor Author

@yxxhero it gives me no result.

However, I ran the tests in my VS Code and saw that TestGenerateID is failing. Do we need to update the hashes manually when we change the contents of ReleaseSpec?

@yxxhero
Copy link
Member

yxxhero commented Aug 19, 2024

@ennekein yeah. you should update.

@yxxhero
Copy link
Member

yxxhero commented Aug 20, 2024

@ennekein oh. BTW. please fix DCO. follow this link: https://github.com/helmfile/helmfile/pull/1672/checks?check_run_id=28971568114


if release.PlainHttp || st.HelmDefaults.PlainHttp || repoPlainHttp {
flags = append(flags, "--plain-http")
} else if release.InsecureSkipTLSVerify || st.HelmDefaults.InsecureSkipTLSVerify || repoSkipTLSVerify {
Copy link
Member

Choose a reason for hiding this comment

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

please don't use else if? maybe we can return directly?

peter-halliday and others added 9 commits August 26, 2024 15:50
accomplished by:

- Adding PlainHttp attribute to RepositorySpec., HelmDefault, ReleaseSpec
- Adding UnitTests for getOCIChart Flags.
- Adding funciton and unitTests for getChartDownload
- Changing and refactoring how Flags are added to getOCIChart.

Resolves helmfile#1224

Signed-off-by: Peter Halliday <peter.halliday@servicenow.com>
Signed-off-by: Pascal Rivard <privard@rbbn.com>
Signed-off-by: Pascal Rivard <privard@rbbn.com>
Signed-off-by: Pascal Rivard <privard@rbbn.com>
Signed-off-by: Pascal Rivard <privard@rbbn.com>
Signed-off-by: Pascal Rivard <privard@rbbn.com>
Signed-off-by: Pascal Rivard <privard@rbbn.com>
Signed-off-by: Pascal Rivard <privard@rbbn.com>
Signed-off-by: Pascal Rivard <privard@rbbn.com>
@yxxhero
Copy link
Member

yxxhero commented Aug 26, 2024

@ennekein LGTM. did you test it locally?

@ennekein
Copy link
Contributor Author

@yxxhero I did build it locally and tested it with both HTTP and HTTPS registries.

@yxxhero yxxhero merged commit 2cc995e into helmfile:main Aug 27, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants