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

fix: gwp does not respect image variant #10603

Merged
merged 6 commits into from
Feb 11, 2025
Merged

fix: gwp does not respect image variant #10603

merged 6 commits into from
Feb 11, 2025

Conversation

davidjumani
Copy link

Description

Ensures the images generated in the gatewayparams via helm respect the global.image.variant

Context

https://github.com/solo-io/solo-projects/issues/7803

Testing steps

Before :

helm-template-oss --set kubeGateway.enabled=true --set global.image.variant=fips-distroless

# Source: gloo/templates/43-gatewayparameters.yaml
kind: GatewayParameters
...
    sdsContainer:
      image:
        registry: quay.io/solo-io
        repository: sds-ee
        tag: 1.0.0-ci1

After :

helm-template-oss --set kubeGateway.enabled=true --set global.image.variant=fips-distroless

# Source: gloo/templates/43-gatewayparameters.yaml
kind: GatewayParameters
...
    sdsContainer:
      image:
        registry: quay.io/solo-io
        repository: sds-ee-fips
        tag: 1.0.0-ci1-distroless

Notes for reviewers

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@solo-changelog-bot
Copy link

Issues linked to changelog:
https://github.com/solo-io/solo-projects/issues/7803

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 10, 2025 •
edited
Loading

Visit the preview URL for this PR (updated for commit b37853c):

https://gloo-edge--pr10603-fix-gwp-fips-distrol-t0l7olu4.web.app

(expires Tue, 18 Feb 2025 15:16:03 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

nfuden
nfuden reviewed Feb 10, 2025
View reviewed changes
install/helm/gloo/templates/_helpers.tpl Outdated
@@ -31,6 +31,7 @@ Construct a container image name from a registry, repository, tag, and digest.
{{- define "gloo.image" -}}
{{- $image := printf "%s/%s" .registry .repository -}}

{{- /* This has been copied over to _gg-helpers.tpl and should be kept in sync */ -}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

why cant we have a shared space for the template to avoid sync issues?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah I was thinking the same thing. I've split the "gloo.image" template into subtemplates to handle each section

install/helm/gloo/templates/_gg-helpers.tpl Outdated
for fips or fips-distroless variants: add -fips to the image repo (name)
*/ -}}
{{- if or $image.fips (has $image.variant (list "fips" "fips-distroless")) -}}
{{- $fipsSupportedImages := list "gloo-ee" "extauth-ee" "gloo-ee-envoy-wrapper" "rate-limit-ee" "discovery-ee" "sds-ee" -}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this include distroless?

Copy link
Author

@davidjumani davidjumani Feb 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yup, the distroless section is separated now!

@davidjumani
Copy link
Author

/kick

Step #1 - "run-tests": Step #7 - "run-tests": �[38;5;10m•�[0m�[38;5;10m•�[0m�[38;5;10m•�[0m�[38;5;10m•�[0m�[38;5;10m•�[0m�[38;5;10m•�[0m�[38;5;10m•�[0m�[38;5;10m•�[0m�[38;5;10m•�[0m �[38;5;10mSUCCESS!�[0m 305.910301ms --- FAIL: TestConsul (0.79s)
Step #1 - "run-tests": Step #7 - "run-tests":     debug.go:32: found unexpected goroutines:
Step #1 - "run-tests": Step #7 - "run-tests":         [Goroutine 127 in state chan send, with github.com/solo-io/gloo/projects/gloo/pkg/plugins/consul.(*plugin).watchEndpointsInDataCenter.func1 on top of the stack:
Step #1 - "run-tests": Step #7 - "run-tests":         github.com/solo-io/gloo/projects/gloo/pkg/plugins/consul.(*plugin).watchEndpointsInDataCenter.func1({0x6fb6f58, 0x4})
Step #1 - "run-tests": Step #7 - "run-tests":         	/workspace/gloo/projects/gloo/pkg/plugins/consul/eds.go:385 +0x6ca
Step #1 - "run-tests": Step #7 - "run-tests":         created by github.com/solo-io/gloo/projects/gloo/pkg/plugins/consul.(*plugin).watchEndpointsInDataCenter in goroutine 132
Step #1 - "run-tests": Step #7 - "run-tests":         	/workspace/gloo/projects/gloo/pkg/plugins/consul/eds.go:311 +0x2ef
Step #1 - "run-tests": Step #7 - "run-tests":         ]
Step #1 - "run-tests": Step #7 - "run-tests": FAIL

@soloio-bulldozer soloio-bulldozer bot merged commit 9d5b077 into main Feb 11, 2025
20 checks passed
@soloio-bulldozer soloio-bulldozer bot deleted the fix-gwp-fips-distroless branch February 11, 2025 16:01
davidjumani added a commit that referenced this pull request Feb 11, 2025
@davidjumani
fix: gwp does not respect image variant (#10603)
2b64c64
@davidjumani davidjumani mentioned this pull request Feb 11, 2025
Merged
4 tasks
davidjumani added a commit that referenced this pull request Feb 11, 2025
@davidjumani
fix: gwp does not respect image variant (#10603)
b3c74d0
@davidjumani davidjumani mentioned this pull request Feb 11, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanrolds ryanrolds ryanrolds approved these changes

@nfuden nfuden nfuden approved these changes

Assignees
No one assigned
Labels
keep pr updated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@davidjumani @ryanrolds @nfuden