Add support for passing external images to helm_chart.images

[bcsgh]

As it stands, publishing a chart with an image not generated by the current module is a bit of a pain. It either needs to be hard code into the chart (which runs afoul of a number of policy enforcement tools) or pulled and then re-pushed. It would be nice to be able to simply directly refence the same label that would be used as a base when creating a derived image.

For example:

MODULE.bazel

oci = use_extension("@rules_oci//oci:extensions.bzl", "oci")
oci.pull(
    name = "python_3_12",
    image = "python:3.12-slim-bullseye",
    ...
)
use_repo(oci, "python_3_12", "python_3_12_linux_amd64")

BUILD

helm_chart(
    name = "thing",
    images = [
        "@python_3_12_linux_amd64",
        ...
    ],
    ...
)

[abrisco]

Could you not add a oci_push target that re-uploads the external image and pass that to images?

[bcsgh]

tl;dr;

• That's exactly the "pulled and then re-pushed" that I referred to as what I want to avoid (or quit doing).
• Upload it where? (Note: rhetorical question, the point being there isn't a generally good answerer.)

The longer version:

Having to include that in the BUILD file is a bunch of boilerplate that seems like it shouldn't be needed and it also only works in the case where the user building the chart has write permissions to the relevant repo, which is generally not the case where the image that should be referenced is a common 3rd party image like I used in the example. Pushing the an image to a different repo raises questions of "why are you asking me to run python from your custom image and not the official one?" and "because Bazel" isn't going to be a satisfactory answer to many consumers. (Particularly not in the current environment of supply chain attacks.)

[abrisco]

What is the issue in referring to an image not tracked by Bazel? I don't think I understand the use case here.

[bcsgh]

I know of at least one common helm lint tool that doesn't allow hard code image names (i.e. it requires they be referenced by SHA). In a similar way, requiring explicit dependency tracking would seem like a reasonable policy for a team to enforce. (Or to put it from the other end, telling your users that that's an unreasonable policy would be out of line.)

FWIW: if this would require support from other rules libraries, I'd not take issue with it being considered dependent on those changes.

[abrisco]

What would prevent you from using the sha directly?

[bcsgh]

Why do you want this to be disallowed? Unless you think it would be something that should be avoided, I don't see what you are getting at.

Re your actual question: As in why not hard code the SHA into the chart? It would be a perfectly reasonable coding standard that all images used by a chart must be provided by the same path, i.e. via helm_chart.images.

Also, just using the SHA would obfuscate what version is being used (the same SHA can have several tags and there isn't any reliable way to know which one is the correct one to track) where as the existing rules for referencing external images already provide a good balance of being explicit about what tag is being requested while still surfacing any tag updates that people might want to know about.

[abrisco]

Why do you want this to be disallowed?

I still don't understand what the issue is so I'm not saying anything should or shouldn't be allowed.

You have a helm chart that uses an external image you specified, and rules_helm is in some way preventing you from publishing that chart? That should definitely be fixed.

Are you saying you want the rules to resolve images despite not having any mechanism to publish said image? I think it'd be fine to support adding things other than a oci_push/img_push target in images assuming the upstream rules have their own mechanism for computing the sha. The following would probably need to be updated in that case if you wanted to take a stab at it
https://github.com/abrisco/rules_helm/blob/a0dd15d1351738d7d2acdda106423533b07611d0/helm/private/helm_package.bzl#L7-L107

Are either of these what you're referring to?

[bcsgh]

... rules_helm is in some way preventing you from publishing that chart?

Yes? When I try to do what I want to work I get this:

ERROR: /.../external/rules_oci++oci+${IMAGE}/BUILD.bazel:5:18: in @@rules_helm+//helm/private:helm_package.bzl%_oci_push_repository_aspect aspect on copy_to_directory rule @@rules_oci++oci+${IMAGE}//:${IMAGE}:
Traceback (most recent call last):
        File "/.../external/rules_helm+/helm/private/helm_package.bzl", line 79, column 13, in _oci_push_repository_aspect_impl
                fail("oci_push/image_push target {} must have a `repository` attribute or a `repository_file` file".format(
Error in fail: oci_push/image_push target @@rules_oci++oci+${IMAGE}//:${IMAGE} must have a `repository` attribute or a `repository_file` file

I'm violating expectation so I'm not really expecting it to work.

Are you saying you want the rules to resolve images despite not having any mechanism to publish said image?

Basically yes (that would get the same result), but not exactly (my motivation is a bit different). I'm not motivated by a lack of ability to push it. Even if I can push it, I wouldn't want to. I want to reference something that was already pushed (by some unspecified means).

[abrisco]

Basically yes (that would get the same result), but not exactly (my motivation is a bit different). I'm not motivated by a lack of ability to push it. Even if I can push it, I wouldn't want to. I want to reference something that was already pushed (by some unspecified means).

Then sounds like the thing you want is solved by updating _oci_push_repository_aspect to work with other rules besides oci_push/img_push which sounds reasonable. This would allow the helm rules to resolve images and simply wouldn't have any push mechanism (as intended). I'd definitely review this change 😄