fix: update v2 Dockerfile and Helm deployment.yaml for correct binary and args - #2524
Draft
fseldow wants to merge 2 commits into
Draft
fix: update v2 Dockerfile and Helm deployment.yaml for correct binary and args#2524fseldow wants to merge 2 commits into
fseldow wants to merge 2 commits into
Conversation
…args - Remove --platform=$BUILDPLATFORM from Dockerfile (breaks non-buildx builds) - Update deployment.yaml command to use /app/ratify-gatekeeper-provider binary - Update args to use Go flag style (-address, -config, -cert-file, -key-file) - Remove v1-specific args (--enable-crd-manager, --cache-*, --metrics-*, --health-port) Signed-off-by: Xinhe Li <xinhl@microsoft.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to align the v2 container build and Helm deployment manifest with the actual v2 Gatekeeper provider binary name and its Go-flag-style CLI, and to remove a Docker build directive that can break non-buildx builds.
Changes:
- Remove
--platform=$BUILDPLATFORMfrom the v2 Dockerfile builder stage. - Update Helm
Deploymentcommand to run/app/ratify-gatekeeper-providerinstead of/app/ratify. - Replace v1-style
serve --http ... -c ...args with v2 flags (-address,-config,-cert-file,-key-file, optional-gatekeeper-ca-cert-file), and remove several v1-only flags.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| Dockerfile | Removes build platform pinning from the builder stage. |
| charts/ratify/templates/deployment.yaml | Updates container command/args to match the v2 provider binary and CLI flags. |
Comments suppressed due to low confidence (1)
charts/ratify/templates/deployment.yaml:81
- This PR removes the v1 --metrics-* args, but the chart still advertises Prometheus scraping via pod annotations and (optionally) exposes a metrics containerPort controlled by .Values.instrumentation.metricsEnabled/metricsPort. The v2 gatekeeper-provider codebase doesn’t implement a metrics endpoint (no metrics handlers), so enabling scraping will produce failing scrapes and a misleading configuration surface. Consider removing/guarding the metrics annotations/port for v2, or implementing a metrics endpoint if it’s still intended to be supported.
{{- if (lookup "v1" "Secret" .Release.Namespace "gatekeeper-webhook-server-cert") }}
- "-gatekeeper-ca-cert-file"
- "/usr/local/tls/client-ca/ca.crt"
{{- end }}
ports:
- containerPort: 6001
{{- if .Values.instrumentation.metricsEnabled }}
- containerPort: {{ required "You must provide .Values.instrumentation.metricsPort" .Values.instrumentation.metricsPort }}
{{- end }}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type: RuntimeDefault | ||
| command: | ||
| - "/app/ratify" | ||
| - "/app/ratify-gatekeeper-provider" |
Comment on lines
+65
to
+72
| - "-address" | ||
| - ":6001" | ||
| - "-c" | ||
| - "-config" | ||
| - "/usr/local/ratify/config.json" | ||
| - "--enable-crd-manager" | ||
| - --cert-dir=/usr/local/tls | ||
| - "-cert-file" | ||
| - "/usr/local/tls/tls.crt" | ||
| - "-key-file" | ||
| - "/usr/local/tls/tls.key" |
Bridge existing v1 auth providers (azureWorkloadIdentity,
azureManagedIdentity, dockerConfig, k8Secrets, etc.) into v2's
registry-store via an adapter that implements ratify-go's
RegistryCredentialGetter interface.
Config example:
"stores": {
"*.azurecr.io": {
"type": "registry-store",
"parameters": {
"authProvider": {
"name": "azureWorkloadIdentity"
}
}
}
}
Static credential still takes precedence if both are specified.
Signed-off-by: Xinhe Li <xinhl@microsoft.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes the v2 Dockerfile and Helm chart
deployment.yamlto match the actual v2 binary and CLI interface. These issues were discovered during hands-on AKS deployment testing ofv2.0.0-alpha.1.Changes
Dockerfile:
--platform=$BUILDPLATFORM— breaks non-buildx builds (e.g.az acr build, plaindocker build)charts/ratify/templates/deployment.yaml:
/app/ratify→/app/ratify-gatekeeper-provider(the actual binary name built bygo build ./cmd/ratify-gatekeeper-provider)serve --http :6001 -c config.json→ v2 Go flag style-address :6001 -config config.json -cert-file ... -key-file ...--enable-crd-manager,--cache-*,--metrics-*,--health-portAdditional issues found during v2 deployment (not addressed in this PR)
/healthzor/readyz— liveness/readiness probes cause kill loopsimage.crdRepositoryto pull CRD image that doesn't exist for v2 — must use--no-hooksand manuallykubectl applyCRDs*.pattern doesn't work as global catch-all — need explicit registry names or*.domainpatternscaBundleTesting
Deployed and tested on AKS cluster (
ratify-v2-test, eastus2) with Gatekeeper v3.22.0. End-to-end image verification flow works after these fixes.Signed-off-by: Xinhe Li xinhl@microsoft.com