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

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
66461e0
fix: gwp does not respect image variant
davidjumani Feb 10, 2025
dbbc6bb
unfocus tests
davidjumani Feb 10, 2025
53c28a6
simplify
davidjumani Feb 11, 2025
51fe37a
fix digest
davidjumani Feb 11, 2025
9fbde6f
Revert "fix digest"
davidjumani Feb 11, 2025
b37853c
really fix digest
davidjumani Feb 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog/v1.19.0-beta7/fix-gwp-fips-distroless.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
changelog:
- type: FIX
issueLink: https://github.com/solo-io/gloo/issues/10602
resolvesIssue: false
description: Fixes the gateway params image to respect the fips and distroless variants specified by global.image.variant

29 changes: 27 additions & 2 deletions install/helm/gloo/templates/_gg-helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,36 @@ ref Image api in projects/gateway2/api/v1alpha1/kube/container.proto
{{- if $image.registry }}
registry: {{ $image.registry }}
{{- end -}}{{/* if $image.registry */}}

{{- /* This has been copied from _helpers.tpl and should be kept in sync */ -}}
{{- if $image.repository }}
repository: {{ $image.repository }}
{{- $repository := $image.repository -}}
{{- /*
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!

{{- if (has $image.repository $fipsSupportedImages) -}}
{{- $repository = printf "%s-fips" $repository -}}
{{- end -}}{{- /* if (has .repository $fipsSupportedImages) */ -}}
{{- end -}}{{- /* if or .fips (has .variant (list "fips" "fips-distroless")) */ -}}
{{ printf "\n" }}
repository: {{ $repository }}
{{- end -}}{{/* if $image.repository */}}

{{- if $image.tag }}
tag: {{ $image.tag }}
{{- $tag := $image.tag -}}
{{- /*
for distroless or fips-distroless variants: add -distroless to the tag
*/ -}}
{{- if has $image.variant (list "distroless" "fips-distroless") -}}
{{- $distrolessSupportedImages := list "gloo" "gloo-envoy-wrapper" "discovery" "sds" "certgen" "kubectl" "access-logger" "ingress" "gloo-ee" "extauth-ee" "gloo-ee-envoy-wrapper" "rate-limit-ee" "discovery-ee" "sds-ee" "observability-ee" "caching-ee" -}}
{{- if (has $image.repository $distrolessSupportedImages) -}}
{{- $tag = printf "%s-distroless" $tag -}} {{- /* Add distroless suffix to the tag since it contains the same binaries in a different container */ -}}
{{- end -}}{{- /* if (has .repository $distrolessSupportedImages) */ -}}
{{- end }}{{- /* if and .tag (has .variant (list "distroless" "fips-distroless")) */ -}}
{{ printf "\n" }}
tag: {{ $tag }}
{{- end -}}{{/* if $image.tag */}}
{{- if $image.digest }}
digest: {{ $image.digest }}
Expand Down
9 changes: 5 additions & 4 deletions install/helm/gloo/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

{{- /*
for fips or fips-distroless variants: add -fips to the image repo (name)
*/ -}}
Expand Down Expand Up @@ -170,7 +171,7 @@ It takes 4 values:
.defaults - the default securityContext for the pod or container
.globalSec - global security settings, usually from .Values.global.securitySettings
.indent - the number of spaces to indent the output. If not set, the output will not be indented.
The indentation argument is necessary because it is possible that no output will be rendered.
The indentation argument is necessary because it is possible that no output will be rendered.
If that happens and the caller handles the indentation the result will be a line of whitespace, which gets caught by the whitespace tests

Depending upon the value of .values.merge, the securityContext will be merged with the defaults or completely replaced.
Expand Down Expand Up @@ -234,7 +235,7 @@ It takes 4 values:
.podSecurityStandards - podSecurityStandard from values.yaml
.globalSec - global security settings, usually from .Values.global.securitySettings
.indent - the number of spaces to indent the output. If not set, the output will not be indented.
The indentation argument is necessary because it is possible that no output will be rendered.
The indentation argument is necessary because it is possible that no output will be rendered.
If that happens and the caller handles the indentation the result will be a line of whitespace, which gets caught by the whitespace tests

If .podSecurityStandards.container.enableRestrictedContainerDefaults is true, the defaults will be set to a restricted set of values.
Expand All @@ -260,7 +261,7 @@ It takes 4 values:
{{- end -}}
{{- /* set default seccompProfileType */ -}}

{{- $pss_restricted_defaults := dict
{{- $pss_restricted_defaults := dict
"runAsNonRoot" true
"capabilities" (dict "drop" (list "ALL"))
"allowPrivilegeEscalation" false }}
Expand All @@ -280,7 +281,7 @@ It takes 4 values:
{{- end -}}
{{- end -}}
{{- /* call general securityContext template */ -}}
{{- include "gloo.securityContext" (dict
{{- include "gloo.securityContext" (dict
"values" $values
"defaults" $defaults
"indent" $indent
Expand Down
51 changes: 51 additions & 0 deletions install/test/k8sgateway_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,57 @@ var _ = Describe("Kubernetes Gateway API integration", func() {
})
})
})

Context("distroless and fips", func() {
DescribeTable("Uses the correct image for the sds-ee container", func(variant string, expectedImage string) {
extraValueArgs := []string{
"kubeGateway.gatewayParameters.glooGateway.sdsContainer.image.registry=my-sds-reg",
"kubeGateway.gatewayParameters.glooGateway.sdsContainer.image.tag=my-sds-tag",
"kubeGateway.gatewayParameters.glooGateway.sdsContainer.image.repository=sds-ee",
"global.image.variant=" + variant,
}
valuesArgs = append(valuesArgs, extraValueArgs...)
// Updated values so need to re-render
prepareHelmManifest(namespace, glootestutils.HelmValues{ValuesArgs: valuesArgs})

gwp := getDefaultGatewayParameters(testManifest)
gwpKube := gwp.Spec.Kube
Expect(gwpKube).ToNot(BeNil())
sdsContainer := gwpKube.SdsContainer.Image
image := fmt.Sprintf("%s/%s:%s", *sdsContainer.Registry, *sdsContainer.Repository, *sdsContainer.Tag)
Expect(image).To(Equal(expectedImage))
},
Entry("No variant specified", "", "my-sds-reg/sds-ee:my-sds-tag"),
Entry("Standard variant", "standard", "my-sds-reg/sds-ee:my-sds-tag"),
Entry("Fips variant", "fips", "my-sds-reg/sds-ee-fips:my-sds-tag"),
Entry("Distroless variant", "distroless", "my-sds-reg/sds-ee:my-sds-tag-distroless"),
Entry("Fips-Distroless variant", "fips-distroless", "my-sds-reg/sds-ee-fips:my-sds-tag-distroless"))

DescribeTable("Uses the correct image for the gloo-ee-envoy-wrapper container", func(variant string, expectedImage string) {
extraValueArgs := []string{
"kubeGateway.gatewayParameters.glooGateway.envoyContainer.image.registry=my-gloo-ee-envoy-wrapper-reg",
"kubeGateway.gatewayParameters.glooGateway.envoyContainer.image.tag=my-gloo-ee-envoy-wrapper-tag",
"kubeGateway.gatewayParameters.glooGateway.envoyContainer.image.repository=gloo-ee-envoy-wrapper",
"global.image.variant=" + variant,
}
valuesArgs = append(valuesArgs, extraValueArgs...)
// Updated values so need to re-render
prepareHelmManifest(namespace, glootestutils.HelmValues{ValuesArgs: valuesArgs})

gwp := getDefaultGatewayParameters(testManifest)
gwpKube := gwp.Spec.Kube
Expect(gwpKube).ToNot(BeNil())
envoyContainer := gwpKube.EnvoyContainer.Image
image := fmt.Sprintf("%s/%s:%s", *envoyContainer.Registry, *envoyContainer.Repository, *envoyContainer.Tag)
Expect(image).To(Equal(expectedImage))
},
Entry("No variant specified", "", "my-gloo-ee-envoy-wrapper-reg/gloo-ee-envoy-wrapper:my-gloo-ee-envoy-wrapper-tag"),
Entry("Standard variant", "standard", "my-gloo-ee-envoy-wrapper-reg/gloo-ee-envoy-wrapper:my-gloo-ee-envoy-wrapper-tag"),
Entry("Fips variant", "fips", "my-gloo-ee-envoy-wrapper-reg/gloo-ee-envoy-wrapper-fips:my-gloo-ee-envoy-wrapper-tag"),
Entry("Distroless variant", "distroless", "my-gloo-ee-envoy-wrapper-reg/gloo-ee-envoy-wrapper:my-gloo-ee-envoy-wrapper-tag-distroless"),
Entry("Fips-Distroless variant", "fips-distroless", "my-gloo-ee-envoy-wrapper-reg/gloo-ee-envoy-wrapper-fips:my-gloo-ee-envoy-wrapper-tag-distroless"))

})
})

When("kube gateway integration is disabled (default)", func() {
Expand Down