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.

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

Added dockerfile path to image label #212

Closed
wants to merge 10 commits into from

Conversation

Josh-01
Copy link

@Josh-01 Josh-01 commented Nov 3, 2020

This change is part of an effort to improve traceability of resources which are built and deployed using GitHub workflows. We want to save the dockerfile path - which is used in building the image - through this action.
Points to note:

  • Instead of the user specifying the path explicitly for this, we will use the path that the action uses to build the image - so no change for the user.
  • This image label can later be used for traceability by any action that is used to deploy the image.

@Josh-01 Josh-01 requested a review from crazy-max as a code owner November 3, 2020 12:27
@Josh-01 Josh-01 force-pushed the AddDockerfilePathToImageLabel branch from e4d4ca8 to 720cd85 Compare November 3, 2020 12:46
@codecov-io
Copy link

codecov-io commented Nov 3, 2020

Codecov Report

Merging #212 (c59c0ce) into master (6efc2b0) will decrease coverage by 0.72%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #212      +/-   ##
==========================================
- Coverage   73.57%   72.84%   -0.73%     
==========================================
  Files           4        4              
  Lines         140      151      +11     
  Branches       24       28       +4     
==========================================
+ Hits          103      110       +7     
  Misses         22       22              
- Partials       15       19       +4     
Impacted Files Coverage Δ
src/context.ts 69.66% <66.66%> (-0.85%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6efc2b0...c59c0ce. Read the comment docs.

@crazy-max
Copy link
Member

@Josh-01 This is already available through the Docker meta action that has been introduced in the Handle tags and labels example:

jobs:
  docker:
    runs-on: ubuntu-latest
    steps:
      -
        name: Checkout
        uses: actions/checkout@v2
      -
        name: Docker meta
        id: docker_meta
        uses: crazy-max/ghaction-docker-meta@v1
        with:
          images: username/repo
      -
        name: Set up Docker Buildx
        uses: docker/setup-buildx-action@v1
      -
        name: Build and push
        id: docker_build
        uses: docker/build-push-action@v2
        with:
          context: .
          file: ./Dockerfile
          platforms: linux/amd64,linux/arm64,linux/386
          push: ${{ github.event_name != 'pull_request' }}
          tags: ${{ steps.docker_meta.outputs.tags }}
          labels: ${{ steps.docker_meta.outputs.labels }}

This will produce the following labels for crazy-max/diun repo for example:

org.opencontainers.image.title=diun
org.opencontainers.image.description=Receive notifications when an image is updated on a Docker registry
org.opencontainers.image.url=https://github.com/crazy-max/diun
org.opencontainers.image.source=https://github.com/crazy-max/diun
org.opencontainers.image.version=edge
org.opencontainers.image.created=2020-11-02T10:32:44.842Z
org.opencontainers.image.revision=7ef743d66080926d00d7c1fbbffcc93feb83abbb
org.opencontainers.image.licenses=MIT

And will be handled through the GitHub package page to display the README of the repo.

@Josh-01 Josh-01 force-pushed the AddDockerfilePathToImageLabel branch from e325116 to 638eda3 Compare November 4, 2020 10:23
Commit sha: 638eda3, Author: Jyotsna
Signed-off-by: Jyotsna <[email protected]>
@Josh-01 Josh-01 force-pushed the AddDockerfilePathToImageLabel branch from 638eda3 to ed4d53f Compare November 4, 2020 10:28
@Josh-01
Copy link
Author

Josh-01 commented Nov 4, 2020

Hi @crazy-max ,

Thanks for pointing to the docker-meta-action. I went through the examples in the README, but while it may seem to be the solution - our use case is slightly different.
The Docker-meta-action enables the user to explicitly generate the standard OCI spec labels for an image and add any custom labels that they want to the image.

In our case , we want to :

  • add the path of the dockerfile ( that was used to build the image) to the image labels,
  • without the user having to configure anything extra as part of the build.

Through this change:
Users who will use the docker-build-action to build their image, will chose which dockerfile to use for the build, and then that choice is saved as a traceability field within the image itself, which can be later used by other actions.

I have edited the labels in my change, into 'source (OCI spec label)' and a custom label 'dockerfilePath' to comply with the OCI specs.

@crazy-max
Copy link
Member

crazy-max commented Nov 4, 2020

Hi @Josh-01,

add the path of the dockerfile ( that was used to build the image) to the image labels,

dockerfilePath is not a standard annotation (as stated in OCI Image Format Specification) and therefore does not seem appropriate to be included in this action. WDYT @tonistiigi?

without the user having to configure anything extra as part of the build.

Sounds good to me to include org.opencontainers.image.source annotation as part of this action. We could also include the revision and created annotations aswell. But this must be done as part of the context module and not the main one. You must also make sure that you do not overwrite user-defined labels through the action labels input. I can work with you on this PR and make the necessary modifications if you wish.

Signed-off-by: Jyotsna <[email protected]>
@Josh-01 Josh-01 force-pushed the AddDockerfilePathToImageLabel branch from 762820b to be3385a Compare November 4, 2020 11:25
@Josh-01
Copy link
Author

Josh-01 commented Nov 4, 2020

Hi @crazy-max ,

Thank you for your inputs - I will make the following changes:

  • Move the changes to the context module from main.
  • Check to ensure user-added labels are not overwritten
  • add revision and created

Is there a some way to include the dockerfile-path in the image labels - as that is one of the key traceability fields we require ?
We should be able to add labels other than the ones specified in the OCI spec , as long as we don't use the OCI spec namespace. Could we discuss adding the dockerfile-path label with a different or no namespace - please do let me know your thoughts on this.

Thanks!

@Josh-01 Josh-01 force-pushed the AddDockerfilePathToImageLabel branch 2 times, most recently from c5deda1 to 5d06edc Compare November 5, 2020 07:01
@Josh-01 Josh-01 force-pushed the AddDockerfilePathToImageLabel branch from 5d06edc to f31af72 Compare November 5, 2020 07:03
@crazy-max
Copy link
Member

@Josh-01 After looking around this feature, it turns out that injecting OCI annotations by default at the creation of the image leads to a confidentiality issue because some private repo are not intended to be disclosed even if the Docker image is public. I'll talk to @tonistiigi about it and we'll give you a feedback.

@Josh-01
Copy link
Author

Josh-01 commented Nov 9, 2020

Hi @crazy-max / @tonistiigi ,

Here's a possible solution reg. the confidentiality issue: we could put an explicit flag input - 'traceData' (FALSE by default, optional),
If it is specified as TRUE by user, only then the labels get added - else not.

We want users to be able use the action for building images and also get the traceability information as seamlessly as possible - which a simple on/off input would effectively be doing.
Please let us know your thoughts on this.

Thanks!

@Josh-01
Copy link
Author

Josh-01 commented Nov 18, 2020

Hi @crazy-max , @tonistiigi ,
Is there any update on this ? ... please let me know if we need to discuss this further.

@ammohant
Copy link

Hi @crazy-max / @tonistiigi ,

Here's a possible solution reg. the confidentiality issue: we could put an explicit flag input - 'traceData' (FALSE by default, optional),
If it is specified as TRUE by user, only then the labels get added - else not.

We want users to be able use the action for building images and also get the traceability information as seamlessly as possible - which a simple on/off input would effectively be doing.
Please let us know your thoughts on this.

Thanks!

Hi @crazy-max / @tonistiigi - What is your view on user providing consent as part of the explicit input provided ? Pls. let us know if you see any issues with this approach.

@ammohant
Copy link

Hi @crazy-max / @tonistiigi - Can you pls let us know if having an explicit flag as input(default NO) helps mitigate the confidentiality concerns ?

@crazy-max
Copy link
Member

crazy-max commented May 26, 2021

Sorry for the huge delay on this PR but after consideration it seems more appropriate for this to be handled upstream. I think the OCI org.opencontainers.image.source annotation is sufficient but it should not be an opt-out model for the user. One could also consider creating a new OCI annotation for Dockerfile tracability like org.opencontainers.image.definition (suggest to open an issue on image-spec repo).

One solution would be to use our metadata action which you could embed into a workflow template if you want to make things as generic as possible.

As I discussed with @clarkbw I think it's better to keep the actions in the current state (login + metadata + qemu + buildx + build-push) and merge them into a single step when nested composite actions will be available. But would need to focus on actions/runner#646 like I said in #208 (comment).

cc @tonistiigi @chris-crone

@crazy-max crazy-max closed this May 26, 2021
@clarkbw
Copy link

clarkbw commented May 26, 2021

One could also consider creating a new OCI annotation for Dockerfile tracability like org.opencontainers.image.definition (suggest to open an issue on image-spec repo).

Agree with this. It would be good to get this discussion going with the OCI group.

For now you could append this as a tag to your existing workflows and still retain the OCI labels like so:

jobs:
  # Push image to GitHub Packages.
  # See also https://docs.docker.com/docker-hub/builds/
  push:

    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v2

      - id: meta
        uses: docker/metadata-action@v3
        with:
          images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}

      - name: Login to ${{ env.REGISTRY }}
        uses: docker/login-action@v1 
        with:
          registry: ${{ env.REGISTRY }}
          username: ${{ github.actor }}
          password: ${{ github.token }}

      - name: Build and push
        uses: docker/build-push-action@v2
        with:
          push: ${{ github.event_name != 'pull_request' }}
          tags: ${{ steps.meta.outputs.tags }}
          labels: |
            ${{ steps.meta.outputs.labels }}
            
            docker-file=https://github.com/github/container-registry-playground.git#Dockerfile

Perhaps you could use a script and output to determine the location of the Dockerfile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants