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.

Sign up for GitHub

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

Jump to bottom

Helm OCI: Add support for TLS registries with self-signed certs #1097

Merged
merged 1 commit into from
Aug 7, 2023
Merged

Helm OCI: Add support for TLS registries with self-signed certs #1097

merged 1 commit into from
Aug 7, 2023

Conversation

souleb
Copy link
Member

@souleb souleb commented May 16, 2023

fixes #723

If implemented user will be able to provide their own custom start and bypass tls verification when interacting with OCI registries over https to pull helmCharts.

@souleb souleb marked this pull request as draft May 16, 2023 16:16
@souleb souleb added enhancement New feature or request area/helm Helm related issues and pull requests area/oci OCI related issues and pull requests labels May 16, 2023
@souleb souleb force-pushed the custom-cert-oci branch from 24c0eed to cd898af Compare May 17, 2023 15:53
@souleb
Copy link
Member Author

souleb commented May 17, 2023

This is currently blocked due to kubernetes/client-go#1245 (comment). Hopefully the next controller-runtime version will fix it.

@souleb souleb force-pushed the custom-cert-oci branch from 36fdfec to fec16c9 Compare May 19, 2023 08:51
stefanprodan
stefanprodan requested changes May 23, 2023
View reviewed changes
api/v1beta2/helmrepository_types.go Outdated Show resolved Hide resolved
@souleb souleb marked this pull request as ready for review May 23, 2023 10:00
@souleb souleb marked this pull request as draft May 23, 2023 10:15
@souleb souleb force-pushed the custom-cert-oci branch from fec16c9 to b66ee24 Compare May 24, 2023 08:37
@stefanprodan stefanprodan changed the title Add support for custom certificate and skip-tls-verify in helm OCI Helm OCI: Add support for TLS registries with self-signed certs May 24, 2023
stefanprodan
stefanprodan reviewed May 24, 2023
View reviewed changes
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

@souleb please rebase and make sure you're using controller-gen v0.12. The changes to the deepcopy files should be undone.

@souleb souleb marked this pull request as ready for review May 25, 2023 08:00
stefanprodan
stefanprodan reviewed May 25, 2023
View reviewed changes
Makefile Outdated Show resolved Hide resolved
docs/api/v1/source.md Outdated Show resolved Hide resolved
docs/api/v1beta2/source.md Outdated Show resolved Hide resolved
@souleb souleb force-pushed the custom-cert-oci branch from e5a46bf to 3adf2e3 Compare May 25, 2023 13:08
darkowlzz
darkowlzz reviewed May 25, 2023
View reviewed changes
docs/spec/v1beta2/helmrepositories.md Outdated Show resolved Hide resolved
internal/controller/helmchart_controller.go Outdated Show resolved Hide resolved
internal/controller/helmrepository_controller_oci.go Outdated Show resolved Hide resolved
internal/controller/helmchart_controller.go Outdated Show resolved Hide resolved
internal/controller/helmrepository_controller_oci.go Outdated Show resolved Hide resolved
internal/controller/helmrepository_controller_oci.go Outdated Show resolved Hide resolved
@souleb
Copy link
Member Author

souleb commented May 30, 2023

@darkowlzz I tried to address your concerns as much as possible.

@souleb souleb force-pushed the custom-cert-oci branch from 5c4bb79 to d36877e Compare May 30, 2023 09:51
@souleb souleb force-pushed the custom-cert-oci branch 2 times, most recently from 0aa4588 to a67929c Compare June 21, 2023 10:01
@aryan9600
Copy link
Member

aryan9600 commented Jun 26, 2023 •
edited
Loading

considering #1131, lets not use .spec.secretRef for this feature and instead use .spec.certSecretRef like OCIRepository does.

@phoban01 phoban01 mentioned this pull request Jul 5, 2023
Closed
@aryan9600 aryan9600 mentioned this pull request Jul 7, 2023
Merged
@souleb souleb force-pushed the custom-cert-oci branch 5 times, most recently from f1db8f0 to 2e8db08 Compare July 31, 2023 14:45
aryan9600
aryan9600 requested changes Aug 1, 2023
View reviewed changes
internal/helm/getter/client_opts.go Outdated Show resolved Hide resolved
internal/helm/getter/client_opts.go Show resolved Hide resolved
internal/helm/getter/client_opts_test.go Outdated Show resolved Hide resolved
internal/controller/suite_test.go Outdated Show resolved Hide resolved
internal/controller/helmchart_controller.go Outdated Show resolved Hide resolved
internal/controller/helmchart_controller_test.go Show resolved Hide resolved
internal/controller/helmchart_controller_test.go Show resolved Hide resolved
internal/controller/helmchart_controller_test.go Outdated Show resolved Hide resolved
internal/controller/helmchart_controller_test.go Show resolved Hide resolved
aryan9600
aryan9600 requested changes Aug 2, 2023
View reviewed changes
Copy link
Member

@aryan9600 aryan9600 left a comment

Choose a reason for hiding this comment

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

its looking good, found mostly nits here and there

internal/helm/getter/client_opts.go Outdated Show resolved Hide resolved
internal/helm/getter/client_opts_test.go Outdated Show resolved Hide resolved
internal/helm/getter/client_opts_test.go Outdated Show resolved Hide resolved
internal/helm/getter/client_opts_test.go Outdated Show resolved Hide resolved
internal/helm/getter/client_opts_test.go Outdated Show resolved Hide resolved
internal/controller/suite_test.go Outdated Show resolved Hide resolved
internal/controller/helmrepository_controller_oci.go Outdated Show resolved Hide resolved
internal/controller/helmrepository_controller_oci.go Outdated Show resolved Hide resolved
internal/controller/helmchart_controller.go Outdated Show resolved Hide resolved
internal/controller/helmchart_controller.go Outdated Show resolved Hide resolved
@stefanprodan stefanprodan mentioned this pull request Aug 2, 2023
Closed
9 tasks
@souleb souleb force-pushed the custom-cert-oci branch 2 times, most recently from d28bc31 to 19eecf8 Compare August 4, 2023 21:09
aryan9600
aryan9600 approved these changes Aug 5, 2023
View reviewed changes
Copy link
Member

@aryan9600 aryan9600 left a comment

Choose a reason for hiding this comment

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

LGTM! thanks @souleb 🙇

darkowlzz
darkowlzz approved these changes Aug 7, 2023
View reviewed changes
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

LGTM!

@souleb @stefanprodan
Add support for custom certificate and skip-tls-verify in helm OCI
d45c08c 
If implemented user will be able to provide their own custom start and
bypass tls verification when interacting with OCI registries over https
to pull helmCharts.

Signed-off-by: Soule BA <soule@weave.works>
@stefanprodan stefanprodan merged commit 6fa8d05 into fluxcd:main Aug 7, 2023
@souleb souleb mentioned this pull request Feb 22, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@somtochiama somtochiama somtochiama left review comments

@hiddeco hiddeco hiddeco left review comments

@aryan9600 aryan9600 aryan9600 approved these changes

@stefanprodan stefanprodan stefanprodan approved these changes

@darkowlzz darkowlzz darkowlzz approved these changes

Assignees
No one assigned
Labels
area/helm Helm related issues and pull requests area/oci OCI related issues and pull requests enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for custom certs in HelmRepositories of Type oci
6 participants
@souleb @aryan9600 @darkowlzz @stefanprodan @hiddeco @somtochiama