-
Notifications
You must be signed in to change notification settings - Fork 23
fix(docker): make Tentacle images OCI-compliant (FD-492) #1259
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.
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
Open
todthomson
wants to merge
6
commits into
main
Choose a base branch
from
eft/tod/fix/FD-492-OCI-compliance-etc
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+100
−40
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
c43f87e
fix(docker): make Tentacle images OCI-compliant (FD-492)
todthomson db2914c
chore(docker): align attestation switch on BUILDX_NO_DEFAULT_ATTESTAT…
todthomson fafe56b
feat(docker): stamp OCI image-spec annotations on the k8s agent index…
todthomson 95ca0c2
docs(docker): add keep-in-sync pointers for OCI image metadata (FD-492)
06aa414
refactor(docker): single build timestamp for k8s label + annotation (…
9cae415
fix(docker): guard against commas in k8s OCI annotation values (FD-492)
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -727,14 +727,68 @@ void BuildAndPushOrLoadKubernetesTentacleContainerImage(bool push, bool load, st | |
| if (includeDebugger) | ||
| tag += "-debug"; | ||
|
|
||
| // Capture one timestamp and reuse it for both the BUILD_DATE build arg (which feeds | ||
| // the Dockerfile's org.opencontainers.image.created LABEL) and the created annotation | ||
| // below, so the image config and the manifest annotation report the same instant. | ||
| var buildDate = DateTime.UtcNow.ToString("O"); | ||
|
|
||
| settings = settings | ||
| .AddBuildArg($"BUILD_NUMBER={FullSemVer}", $"BUILD_DATE={DateTime.UtcNow:O}", $"RuntimeDepsTag={runtimeDepsImageTag}") | ||
| .AddBuildArg($"BUILD_NUMBER={FullSemVer}", $"BUILD_DATE={buildDate}", $"RuntimeDepsTag={runtimeDepsImageTag}") | ||
| .SetPlatform(DockerPlatform) | ||
| .SetTag(tag) | ||
| .SetFile(dockerfile) | ||
| .SetPath(RootDirectory) | ||
| .SetPush(push) | ||
| .SetLoad(load); | ||
| .SetPath(RootDirectory); | ||
|
|
||
| if (push) | ||
| { | ||
| // FD-492: Force a single, consistent OCI media type across the whole image | ||
| // manifest and disable default attestations. Without this, buildx can emit | ||
| // a manifest that mixes Docker and OCI layer media types (and an attestation | ||
| // index), which strict OCI clients such as Podman reject - in particular when | ||
| // the image is used as a base image (FROM ...). BUILDX_NO_DEFAULT_ATTESTATIONS | ||
| // is used (rather than --provenance=false) so the same switch applies to both | ||
| // `docker buildx build` here and `docker buildx bake` in the TeamCity Linux | ||
| // image build (see OctopusDeploy/TeamCity-Configuration). | ||
| // | ||
| // We also stamp the OCI image-spec annotations (org.opencontainers.image.*) | ||
| // onto both the image index and each platform manifest. The Dockerfile LABELs | ||
| // set the same metadata on the image *config*; annotations are the spec-preferred | ||
| // location that registries and OCI tooling read from the manifest/index. | ||
| // Keep these values in sync with the org.opencontainers.image.* LABELs in | ||
| // docker/kubernetes-agent-tentacle/Dockerfile. | ||
| var annotations = new Dictionary<string, string> | ||
| { | ||
| ["org.opencontainers.image.title"] = "Octopus Deploy Kubernetes Agent Tentacle", | ||
| ["org.opencontainers.image.vendor"] = "Octopus Deploy", | ||
| ["org.opencontainers.image.url"] = "https://octopus.com", | ||
| ["org.opencontainers.image.source"] = "https://github.com/OctopusDeploy/OctopusTentacle", | ||
| ["org.opencontainers.image.licenses"] = "Apache-2.0", | ||
| ["org.opencontainers.image.description"] = "Octopus Kubernetes Agent Tentacle instance with auto-registration to Octopus Server", | ||
| ["org.opencontainers.image.version"] = FullSemVer, | ||
| ["org.opencontainers.image.created"] = buildDate, | ||
| }; | ||
|
|
||
| // annotation-index.* lands on the image index, annotation.* on each platform manifest. | ||
| // Guard: annotation values are folded into the comma-separated buildx --output option | ||
| // list, so a comma in a value would corrupt it (the keys are fixed and comma-free). | ||
| var output = "type=image,oci-mediatypes=true,push=true"; | ||
| foreach (var (key, value) in annotations) | ||
| { | ||
| if (value.Contains(',')) | ||
| throw new InvalidOperationException( | ||
| $"OCI annotation '{key}' value must not contain a comma; it would break the buildx --output option list: '{value}'"); | ||
|
|
||
| output += $",annotation-index.{key}={value},annotation.{key}={value}"; | ||
| } | ||
|
Comment on lines
+771
to
+782
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are controlling the annotations above and |
||
|
|
||
| settings = settings | ||
| .SetOutput(output) | ||
| .AddProcessEnvironmentVariable("BUILDX_NO_DEFAULT_ATTESTATIONS", "1"); | ||
| } | ||
| else | ||
| { | ||
| settings = settings.SetLoad(load); | ||
| } | ||
|
|
||
| if (includeDebugger) | ||
| { | ||
|
|
||
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
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Claude is over commenting - can we trim it down to things that future people will need when looking at the code.
Reasoning for "why are we making the changes" are much better as comments on a PR IMHO.