Skip to content

ci: use docker/metadata-action to gather tags/labels#3998

Merged
v1v merged 4 commits intoelastic:mainfrom
v1v:feature/provenance-docker-push
May 13, 2024
Merged

ci: use docker/metadata-action to gather tags/labels#3998
v1v merged 4 commits intoelastic:mainfrom
v1v:feature/provenance-docker-push

Conversation

@v1v
Copy link
Copy Markdown
Member

@v1v v1v commented Apr 29, 2024 •
edited
Loading

What

Use https://github.com/docker/metadata-action/tree/8e5442c4ef9f78752691e2d8f8d19755c6f78e81/?tab=readme-ov-file#semver to get the tag version and open-container labels.

Use sha commit for the GitHub action related to docker.

Test

See https://github.com/elastic/apm-agent-nodejs/actions/runs/8878357355, didn't run with push

Run docker/build-push-action@2cdde995de11925a030ce8070c3d77a52ffcf1c0
  with:
    context: .
    push: false
    tags: docker.elastic.co/observability/apm-agent-nodejs:test-latest
    labels: org.opencontainers.image.created=[2](https://github.com/elastic/apm-agent-nodejs/actions/runs/8878357355/job/24373834641#step:9:2)024-04-29T12:16:2[3](https://github.com/elastic/apm-agent-nodejs/actions/runs/8878357355/job/24373834641#step:9:3).000Z
  org.opencontainers.image.description=Elastic APM Node.js Agent
  org.opencontainers.image.licenses=BSD-2-Clause
  org.opencontainers.image.revision=f9f28681ae69736728d1[5](https://github.com/elastic/apm-agent-nodejs/actions/runs/8878357355/job/24373834641#step:9:5)aaa93880fffba5f2e6d
  org.opencontainers.image.source=https://github.com/elastic/apm-agent-nodejs
  org.opencontainers.image.title=apm-agent-nodejs
  org.opencontainers.image.url=https://github.com/elastic/apm-agent-nodejs
  org.opencontainers.image.version=test-latest
    build-args: AGENT_DIR=/build/dist/nodejs

Checklist

  • Implement code
  • Add tests
  • Update TypeScript typings
  • Update documentation
  • Add CHANGELOG.asciidoc entry
  • Commit message follows commit guidelines

@v1v v1v marked this pull request as ready for review April 29, 2024 12:17
@v1v v1v requested review from a team April 29, 2024 12:17
@v1v v1v self-assigned this Apr 29, 2024
@v1v v1v mentioned this pull request Apr 29, 2024
Merged
v1v
v1v commented Apr 29, 2024
View reviewed changes
trentm
trentm reviewed Apr 29, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@trentm trentm left a comment

Choose a reason for hiding this comment

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

Use https://github.com/docker/metadata-action/tree/8e5442c4ef9f78752691e2d8f8d19755c6f78e81/?tab=readme-ov-file#typeref to get the tags/annotatiions/labels

You link to #typeref there, and link to:

See https://github.com/elastic/apm-agent-nodejs/actions/runs/8878357355

which does show using a config of the "docker-metadata" action with:

type=ref,event=tag,prefix=,suffix=

However, the current diff in this PR does not use type=ref,... at all, so I am confused.

.github/workflows/release.yml
${{ env.DOCKER_IMAGE_NAME }}:${{ steps.docker-vars.outputs.tag }}
${{ env.DOCKER_IMAGE_NAME }}:${{ steps.docker-vars.outputs.latest }}
tags: ${{ steps.docker-meta.outputs.tags }}
labels: ${{ steps.docker-meta.outputs.labels }}
Copy link
Copy Markdown
Member

@trentm trentm Apr 29, 2024

Choose a reason for hiding this comment

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

For the record this results in adding a number of labels that we never had before:

Before:

/usr/bin/docker buildx build --build-arg AGENT_DIR=/build/dist/nodejs --iidfile /home/runner/work/_temp/docker-actions-toolkit-fyNsIK/iidfile --tag docker.elastic.co/observability/apm-agent-nodejs:test --tag docker.elastic.co/observability/apm-agent-nodejs:test-latest --metadata-file /home/runner/work/_temp/docker-actions-toolkit-fyNsIK/metadata-file --push .

after:

/usr/bin/docker buildx build --build-arg AGENT_DIR=/build/dist/nodejs --iidfile /home/runner/work/_temp/docker-actions-toolkit-zpfy5E/iidfile --label org.opencontainers.image.created=2024-04-29T12:16:23.000Z --label org.opencontainers.image.description=Elastic APM Node.js Agent --label org.opencontainers.image.licenses=BSD-2-Clause --label org.opencontainers.image.revision=f9f28681ae69736728d15aaa93880fffba5f2e6d --label org.opencontainers.image.source=https://github.com/elastic/apm-agent-nodejs --label org.opencontainers.image.title=apm-agent-nodejs --label org.opencontainers.image.url=https://github.com/elastic/apm-agent-nodejs --label org.opencontainers.image.version=test-latest --tag docker.elastic.co/observability/apm-agent-nodejs:test-latest --metadata-file /home/runner/work/_temp/docker-actions-toolkit-zpfy5E/metadata-file .

Copy link
Copy Markdown
Member Author

@v1v v1v Apr 29, 2024

Choose a reason for hiding this comment

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

Those are the opencontainer default labels:

We can remove them, sorry if I was not clear about this change

Copy link
Copy Markdown
Member

@trentm trentm Apr 29, 2024

Choose a reason for hiding this comment

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

I don't have a strong argument either way. If we add these, then they somewhat become a promise that we will always define them in the future. I don't think it likely that that will be an issue.

On balance, I would be inclined to not add these labels unless there is a known use case to have them. I am fine either way however.

@v1v
Copy link
Copy Markdown
Member Author

v1v commented Apr 29, 2024

However, the current diff in this PR does not use type=ref,... at all, so I am confused.

I initially used but discovered that the type: ref uses the name of the tag and the tags in this particular case use the format v<version>, therefore the image name will contain v<version> rather than <version>

Those details were mentioned in a similar PR that I raised in the apm-agent-python:

reakaleek
reakaleek previously approved these changes May 6, 2024
View reviewed changes
@v1v v1v mentioned this pull request May 10, 2024
Merged
6 tasks
@v1v v1v merged commit 041109a into elastic:main May 13, 2024
v1v added a commit to v1v/apm-agent-nodejs that referenced this pull request May 13, 2024
@v1v
Merge remote-tracking branch 'upstream/main' into feature/docker-imag…
3d8c35b 
…es-wolfi

* upstream/main:
  ci: use docker/metadata-action to gather tags/labels (elastic#3998)
  chore(deps): bump @opentelemetry/exporter-prometheus (elastic#4016)
  chore(deps): bump @opentelemetry/sdk-metrics to 1.24.1 in test fixtures (elastic#4015)
  chore(deps): bump @opentelemetry/sdk-metrics from 1.24.0 to 1.24.1 (elastic#4014)
  chore(deps-dev): bump azure-functions-core-tools (elastic#4017)
  chore(deps-dev): bump undici from 6.15.0 to 6.16.1 (elastic#4013)
  chore(deps-dev): bump fastify from 4.26.2 to 4.27.0 (elastic#4012)
  chore(deps): bump pino from 8.17.1 to 8.21.0 (elastic#4011)
  chore(deps): bump @opentelemetry/core from 1.24.0 to 1.24.1 (elastic#4010)
  chore(deps-dev): bump mongodb from 6.6.0 to 6.6.1 (elastic#4009)
  chore(deps-dev): bump eslint-plugin-n in the eslint group (elastic#4008)
v1v added a commit that referenced this pull request May 13, 2024
@v1v
Merge branch 'feature/docker-images-wolfi' into test-wolfi
2a584e4 
* feature/docker-images-wolfi:
  ci: use docker/metadata-action to gather tags/labels (#3998)
  chore(deps): bump @opentelemetry/exporter-prometheus (#4016)
  chore(deps): bump @opentelemetry/sdk-metrics to 1.24.1 in test fixtures (#4015)
  chore(deps): bump @opentelemetry/sdk-metrics from 1.24.0 to 1.24.1 (#4014)
  chore(deps-dev): bump azure-functions-core-tools (#4017)
  chore(deps-dev): bump undici from 6.15.0 to 6.16.1 (#4013)
  chore(deps-dev): bump fastify from 4.26.2 to 4.27.0 (#4012)
  chore(deps): bump pino from 8.17.1 to 8.21.0 (#4011)
  chore(deps): bump @opentelemetry/core from 1.24.0 to 1.24.1 (#4010)
  chore(deps-dev): bump mongodb from 6.6.0 to 6.6.1 (#4009)
  chore(deps-dev): bump eslint-plugin-n in the eslint group (#4008)
trentm pushed a commit that referenced this pull request May 13, 2024
@v1v @trentm
ci: use docker/metadata-action to gather tags/labels (#3998)
4e508e2
@trentm trentm mentioned this pull request May 13, 2024
Merged
PeterEinberger pushed a commit to fpm-git/apm-agent-nodejs that referenced this pull request Aug 20, 2024
@v1v @PeterEinberger
ci: use docker/metadata-action to gather tags/labels (elastic#3998)
1a996e6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@trentm trentm trentm approved these changes

+1 more reviewer

@reakaleek reakaleek reakaleek left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@v1v v1v

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@v1v @trentm @reakaleek