Describe the bug
Changes made via #19347 make a bad assumption, specifically https://github.com/grafana/loki/pull/19347/changes#diff-2296a17a0f4611b539d59e3c89cf6a8f53db863af266d6fdab2846d704c47b52R177-R179 assumes that repository names will not include a . which is not a safe assumption. This introduced a breaking change.
To Reproduce
Steps to reproduce the behavior:
Checkout current code base: 328f3d45b99381cf2383332e5f9ae08969c06d5b and compare the following three template commands:
± helm template . --set loki.storage.bucketNames.chunks=foo --set loki.storage.bucketNames.ruler=foo --set loki.useTestSchema=true --set loki.image.registry=my.privaterepo.com --set loki.image.repository=foo/loki-fips --set loki.image.tag=v3.6.4 --set lokiCanary.image.registry=my.privaterepo.com --set lokiCanary.image.repository=foo/loki-canary-fips --set lokiCanary.image.tag=v3.6.4 | grep image: | grep fips
image: my.privaterepo.com/foo/loki-canary-fips:v3.6.4
image: my.privaterepo.com/foo/loki-fips:v3.6.4
image: my.privaterepo.com/foo/loki-fips:v3.6.4
image: my.privaterepo.com/foo/loki-fips:v3.6.4
± helm template . --set loki.storage.bucketNames.chunks=foo --set loki.storage.bucketNames.ruler=foo --set loki.useTestSchema=true --set loki.image.registry=my.privaterepo.com --set loki.image.repository=foo.com/loki-fips --set loki.image.tag=v3.6.4 --set lokiCanary.image.registry=my.privaterepo.com --set lokiCanary.image.repository=foo.com/loki-canary-fips --set lokiCanary.image.tag=v3.6.4 | grep image: | grep fips
image: foo.com/loki-canary-fips:v3.6.4
image: foo.com/loki-fips:v3.6.4
image: foo.com/loki-fips:v3.6.4
image: foo.com/loki-fips:v3.6.4
± helm template . --set loki.storage.bucketNames.chunks=foo--set loki.storage.bucketNames.ruler=foo--set loki.useTestSchema=true --set loki.image.registry=my.privaterepo.com --set loki.image.repository=foo.com/loki-fips --set loki.image.tag=v3.6.4 --set lokiCanary.image.registry=my.privaterepo.com --set lokiCanary.image.repository=foo.com/loki-canary-fips --set lokiCanary.image.tag=v3.6.4 --set global.imageRegistry=my.privaterepo.com | grep image: | grep fips
image: foo.com/loki-canary-fips:v3.6.4
image: foo.com/loki-fips:v3.6.4
image: foo.com/loki-fips:v3.6.4
image: foo.com/loki-fips:v3.6.4
You will see in the second and third examples that because the repository includes a segment with an FQDN like string it assumes that the registry should not be included in the image name when in fact it should be as the appropriate place to pull the images from are my.private.repo.com/foo.com/loki-...
Expected behavior
I expect the chart to maintain compatibility between minor version bumps (This issue didn't exist in 6.49.0 and is broken in 6.51.0). I also expect the chart to honor valid image path URLs even though they may be less common. An image ref of my.private.repo.com/foo.com/loki-canary-fips:v.3.6.4 segmented as:
image:
registry: my.private.repo.com
repository: foo.com/loki-canary-fips
tag: v.3.6.4
is completely valid.
You may ask why don't you provide the values like:
± helm template . --set loki.storage.bucketNames.chunks=foo --set loki.storage.bucketNames.ruler=foo --set loki.useTestSchema=true --set loki.image.registry=my.privaterepo.com --set loki.image.repository=loki-fips --set loki.image.tag=v3.6.4 --set lokiCanary.image.registry=my.privaterepo.com --set lokiCanary.image.repository=loki-canary-fips --set lokiCanary.image.tag=v3.6.4 --set global.imageRegistry=my.privaterepo.com/foo.com | grep image: | grep fips
image: my.privaterepo.com/foo.com/loki-canary-fips:v3.6.4
image: my.privaterepo.com/foo.com/loki-fips:v3.6.4
image: my.privaterepo.com/foo.com/loki-fips:v3.6.4
image: my.privaterepo.com/foo.com/loki-fips:v3.6.4
The reason is because we use tools like renovatebot which rely on scanning the helm chart files within source code to look for updates and it relies on proper specification across these three fields to reliably lookup potential upstream changes:
image:
registry:
repository:
tag:
Environment:
- Infrastructure: laptop
- Deployment tool: helm
Describe the bug
Changes made via #19347 make a bad assumption, specifically https://github.com/grafana/loki/pull/19347/changes#diff-2296a17a0f4611b539d59e3c89cf6a8f53db863af266d6fdab2846d704c47b52R177-R179 assumes that repository names will not include a
.which is not a safe assumption. This introduced a breaking change.To Reproduce
Steps to reproduce the behavior:
Checkout current code base:
328f3d45b99381cf2383332e5f9ae08969c06d5band compare the following three template commands:You will see in the second and third examples that because the repository includes a segment with an FQDN like string it assumes that the registry should not be included in the image name when in fact it should be as the appropriate place to pull the images from are
my.private.repo.com/foo.com/loki-...Expected behavior
I expect the chart to maintain compatibility between minor version bumps (This issue didn't exist in
6.49.0and is broken in6.51.0). I also expect the chart to honor valid image path URLs even though they may be less common. An image ref ofmy.private.repo.com/foo.com/loki-canary-fips:v.3.6.4segmented as:is completely valid.
You may ask why don't you provide the values like:
The reason is because we use tools like
renovatebotwhich rely on scanning the helm chart files within source code to look for updates and it relies on proper specification across these three fields to reliably lookup potential upstream changes:Environment: