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

Support 3rd party Helm artifacts in Helm deployments #16478

Merged
merged 4 commits into from Aug 15, 2022

Conversation

alonsodomin
Copy link
Contributor

Closes #16477

Copy link
Sponsor Contributor

@benjyw benjyw left a comment

Choose a reason for hiding this comment

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

Nice! I had one comment which is not required, just an idea.

src/python/pants/backend/helm/util_rules/chart.py Outdated Show resolved Hide resolved
release_name = request.field_set.release_name.value or request.field_set.address.target_name
release_name = (
request.field_set.release_name.value
or request.field_set.address.target_name.replace("_", "-")
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Is this a Helm convention to use dashes not underscores?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, and it's also forbidden to use underscores to name a release, so the easy way out was transforming them into dashes...

@alonsodomin alonsodomin merged commit eec449f into pantsbuild:main Aug 15, 2022
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Exciting! Thanks!

```python src/deploy/BUILD
helm_deployment(
name="main",
dependencies=["//3rdparty/helm/jetstack:cert-manager"],
Copy link
Contributor

Choose a reason for hiding this comment

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

How hard would it be to add dependency inference for helm_artifacts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the deployments is not possible to infer the chart, first or third party. Deployments are basically a set of YAML files and settings with no info on what chart they are meant to be used on.

cczona pushed a commit to cczona/pants that referenced this pull request Sep 1, 2022
Allows referencing a `helm_artifact` target in the `dependencies` field of a `helm_deployment`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support using 3rd party Helm artifacts in deployments
3 participants