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

v3 release #84

Open
sondrelg opened this issue Jun 24, 2024 · 21 comments
Open

v3 release #84

sondrelg opened this issue Jun 24, 2024 · 21 comments

Comments

@sondrelg
Copy link
Member

If you notice any problems in the new release, please post them here 👍

@ubbeK
Copy link

ubbeK commented Jun 25, 2024

Will there be a v3 tag?

@ubbeK
Copy link

ubbeK commented Jun 25, 2024

Will there be a v3 tag?

Sorry, didn't read the release notes too carefully

We'll no longer maintain mutable major and minor version tags for the action. There will be no v3 target for the action, just v3.0.0 and other exact versions.

@sondrelg
Copy link
Member Author

Will there be a v3 tag?

Sorry, didn't read the release notes too carefully

We'll no longer maintain mutable major and minor version tags for the action. There will be no v3 target for the action, just v3.0.0 and other exact versions.

No worries ☺️ I could be convinced otherwise, but what we've done in the past is just tag every patch release with the equivalent of v3, which in practice means everyone's on latest, and means you break everyone if you introduce an issue, which is inevitable.

Dropping the mutable tags seems like it should:

  • Give us more time to catch and fix new issues, before they reach too many people
  • Makes it less likely that users will experience an issue, since it'll likely be fixed before most people upgrade

If you're worried about falling behind on action versions, I can recommend using dependabot for github actions, like we do here 👍 It will open a PR with the relevant release notes when there's a new action version available.

Let me know if this misses out on some other benefit of a v3 tag that I haven't considered!

@ubbeK
Copy link

ubbeK commented Jun 25, 2024

Everybody does it differently regarding tags and mutable tags. I personally prefer mutable tags for major-tag and minor-tag (examples v3, v3.1) which gives the freedom for the end user to choose what kind of tag to use. As long as SemVer 2.0 is followed regarding the interface (input/output variable names and type/format), then I wouldn't effectively be targeting the main branch when using the major mutable tag. At the same time, I could target minor mutable tag to lock in on a specific state, but still get all patch fixes.

I am personally maintaining a few github actions and reusable github action workflows which are located in several private git repos, and I would like to share my workflow for PR merges which implements what I discussed above. I am not saying that you should implement it yourself, as I get that it is hard to test all scenarios for a small/medium sized opensource project.

on:
  pull_request: # on merge to main only, see if-statement: github.event.pull_request.merged == true
    branches:
    - main
    types:
    - closed

jobs:
  pr_merge:
    if: github.event.pull_request.merged == true
    runs-on: ubuntu-latest
    steps:
    - name: Checkout repo
      uses: actions/checkout@v4
      with:
        fetch-depth: 0 # due to paulhatch/semantic-version step below

    - name: Get next semver version
      id: get_version
      uses: paulhatch/[email protected]
      with:
        tag_prefix: "v"
        version_format: "${major}.${minor}.${patch}"
        major_pattern: "(MAJOR)"
        minor_pattern: "(MINOR)"
        search_commit_body: true

    - name: Create release
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      run: |
        gh release create "${{ steps.get_version.outputs.version_tag }}" \
          --repo="$GITHUB_REPOSITORY" \
          --title="${{ steps.get_version.outputs.version_tag }}" \
          --generate-notes

        git tag --force v${{ steps.get_version.outputs.major }}
        git push origin v${{ steps.get_version.outputs.major }} --force

        git tag --force v${{ steps.get_version.outputs.major }}.${{ steps.get_version.outputs.minor }}
        git push origin v${{ steps.get_version.outputs.major }}.${{ steps.get_version.outputs.minor }} --force

    - name: PR comment
      uses: actions/github-script@v7
      with:
        retries: 3
        script: |
          github.rest.issues.createComment({
            issue_number: context.issue.number,
            owner: context.repo.owner,
            repo: context.repo.repo,
            body: 'Release [${{ steps.get_version.outputs.version_tag }}](https://github.com/${{ github.repository }}/releases/tag/${{ steps.get_version.outputs.version_tag }}) created on SHA ${{ github.sha }}.\n\n' +
                  'Tag [v${{ steps.get_version.outputs.major }}](https://github.com/${{ github.repository }}/releases/tag/v${{ steps.get_version.outputs.major }}) created on SHA ${{ github.sha }}.\n\n' +
                  'Tag [v${{ steps.get_version.outputs.major }}.${{ steps.get_version.outputs.minor }}](https://github.com/${{ github.repository }}/releases/tag/v${{ steps.get_version.outputs.major }}.${{ steps.get_version.outputs.minor }}) created on SHA ${{ github.sha }}.'
          })

Keep up the good work, and I will definitely check out the dependabot example you supplied. Thanks!

@raszi
Copy link

raszi commented Jun 25, 2024

I am having issues defining a rule that would remove every outdated image but the skipped ones. The previous config looked like this:

image-names: foo
cut-off: One month ago UTC
keep-at-least: 1
account-type: personal
skip-tags: base, production
token: ${{ secrets.GITHUB_TOKEN }}
token-type: github-token

The new:

account: user
token: ${{ secrets.GITHUB_TOKEN }}
image-names: foo
image-tags: *, !base, !production
cut-off: 4w
keep-n-most-recent: 1

But this does not work. GitHub says: Invalid workflow file because of invalid syntax on the image-tags line.

@sondrelg
Copy link
Member Author

sondrelg commented Jun 25, 2024

Please try image-tags: !base, !production or image-tags: !base !production @raszi 👍 The wildcard is implicit as long as no positive matchers (expressions without ! prefixed) are specified.

I'll see if I can fix this in a patch release, so it would work in the future.

@raszi
Copy link

raszi commented Jun 25, 2024

That does not work either. 😔 Now it is complaining about the with: line. With or without commas, it has the same issue.

@sondrelg
Copy link
Member Author

See this example which does the same thing.

Is your workflow public?

@raszi
Copy link

raszi commented Jun 25, 2024

That example is not exactly the same, because that has an explicit wildcard. With the explicit wildcard, I get the error when I use *, as I pasted above. Without the explicit wildcard, I get the error on the with: level. If I use an explicit wildcard that looks like the example you linked (like t*), it has no errors, but that is not the same behavior.

No, unfortunately, the workflow is not public.

@ubbeK
Copy link

ubbeK commented Jun 25, 2024

That example is not exactly the same, because that has an explicit wildcard. With the explicit wildcard, I get the error when I use *, as I pasted above. Without the explicit wildcard, I get the error on the with:. level. If I use an explicit wildcard that looks like the example you linked (like t*), it has no errors, but that is not the same behavior.

No, unfortunately, the workflow is not public.

I would guess that you need to make the yaml property it into a string to not get that with error, like

account: user
token: ${{ secrets.GITHUB_TOKEN }}
image-names: foo
image-tags: "!base !production"
cut-off: 4w
keep-n-most-recent: 1

@raszi
Copy link

raszi commented Jun 25, 2024

I am unsure how the YAML value is handled by this project, but will that work the same way? Wouldn't it try to ignore images with tags base !production?

@sondrelg
Copy link
Member Author

sondrelg commented Jun 25, 2024

It should work the same way, but I'd suggest running the action with dry-run: true if you want to be sure. It'd be great if you could share the entire step definition, so it's possible to replicate the issue.

@raszi
Copy link

raszi commented Jun 25, 2024

This is my current step definition:

  clean-old-images:
    name: Delete old unused images
    runs-on: ubuntu-latest

    steps:
      - name: Delete old images
        uses: snok/[email protected]
        with:
          account: user
          token: ${{ secrets.GITHUB_TOKEN }}
          image-names: foo
          image-tags: "!base !production"
          cut-off: 4w
          keep-n-most-recent: 1

@sondrelg
Copy link
Member Author

sondrelg commented Jun 25, 2024

That should work @raszi 👍 Quoting the values is fine. I see what you mean about it being illegal yaml. I'll update the docs and release note.

@sondrelg
Copy link
Member Author

As a quick tip: you can run the action with rust-log: container_retention_policy=trace to get a more detailed look at what's going on. See the Delete old images step in https://github.com/snok/container-retention-policy/actions/runs/9664048758/job/26657757575 for an example

@amoosbr
Copy link

amoosbr commented Jul 29, 2024

Hi,
not sure if my question belongs to the v3 release or not.
Just wanted to let you know, about my experience, when I tried to use v3.0.0 with a GitHub App token.

Based on the README image-names section, I expected to use a GitHub App for cleanup a image name like bla/*
Readme snippet:

These operators are only available for personal- and GitHub app-tokens. See the token parameter section for more info.

Unfortunately, when I tried to use it as documented in the GitHub App token sample, I got the following response:

2024-07-29T09:34:18.768949Z DEBUG container_retention_policy: Logging initialized
2024-07-29T09:34:18.769670Z DEBUG parse input: container_retention_policy::cli::models: Recognized token as temporal token
2024-07-29T09:34:18.770018Z DEBUG parse input: container_retention_policy::client::builder: Constructing base urls
2024-07-29T09:34:18.770047Z DEBUG parse input: container_retention_policy::client::builder: Constructing HTTP headers
2024-07-29T09:34:18.770065Z DEBUG parse input: container_retention_policy::client::builder: Creating rate-limited services
2024-07-29T09:34:18.770328Z DEBUG fetch rate limit: container_retention_policy::client::client: Retrieving Github API rate limit
2024-07-29T09:34:18.842278Z DEBUG fetch rate limit: container_retention_policy::client::client: There are [15](https://github.com/ORG/actions/actions/runs/10141744949/job/28039764262#step:4:16)000 requests remaining in the rate limit
thread 'main' panicked at src/client/client.rs:46:17:
Restrictions in the Github API prevent us from listing packages when using a $GITHUB_TOKEN token. Because of this, filtering with '!' and '*' are not supported for this token type. Image name cache/* is therefore not valid.

Looking at the latest history, I saw a commit, that mentions GitHub App tokens behave like temporal tokens.
I tried using building and running the 3.0.0 release candidate, where the GitHub App tokens was not treated as temporal token. Then my GitHub App token didn't have the needed scopes:

The token does not have the scopes needed. Tokens need `read:packages` and `delete:packages`. The scopes found were none.

My GitHub App has packages:write permission and is installed on the org with access to all repositories. I didn't find a packages:delete option for GitHub Apps.

I'm not 100% sure, if I just used the action wrong or my App has wrong settings. But if it is not possible to have wildcard image names with GitHub App tokens, perhaps the README can be updated.

As soon as I switched to using a classical PAT, the workflow stated to work as expected.

@raszi
Copy link

raszi commented Jul 29, 2024

Quoting the values is fine.

Yes, this worked! Thanks!

@sondrelg
Copy link
Member Author

Hi, not sure if my question belongs to the v3 release or not. Just wanted to let you know, about my experience, when I tried to use v3.0.0 with a GitHub App token.

Based on the README image-names section, I expected to use a GitHub App for cleanup a image name like bla/* Readme snippet:

These operators are only available for personal- and GitHub app-tokens. See the token parameter section for more info.

Unfortunately, when I tried to use it as documented in the GitHub App token sample, I got the following response:

2024-07-29T09:34:18.768949Z DEBUG container_retention_policy: Logging initialized
2024-07-29T09:34:18.769670Z DEBUG parse input: container_retention_policy::cli::models: Recognized token as temporal token
2024-07-29T09:34:18.770018Z DEBUG parse input: container_retention_policy::client::builder: Constructing base urls
2024-07-29T09:34:18.770047Z DEBUG parse input: container_retention_policy::client::builder: Constructing HTTP headers
2024-07-29T09:34:18.770065Z DEBUG parse input: container_retention_policy::client::builder: Creating rate-limited services
2024-07-29T09:34:18.770328Z DEBUG fetch rate limit: container_retention_policy::client::client: Retrieving Github API rate limit
2024-07-29T09:34:18.842278Z DEBUG fetch rate limit: container_retention_policy::client::client: There are [15](https://github.com/bosch-adas-genai/actions/actions/runs/10141744949/job/28039764262#step:4:16)000 requests remaining in the rate limit
thread 'main' panicked at src/client/client.rs:46:17:
Restrictions in the Github API prevent us from listing packages when using a $GITHUB_TOKEN token. Because of this, filtering with '!' and '*' are not supported for this token type. Image name cache/* is therefore not valid.

Looking at the latest history, I saw a commit, that mentions GitHub App tokens behave like temporal tokens. I tried using building and running the 3.0.0 release candidate, where the GitHub App tokens was not treated as temporal token. Then my GitHub App token didn't have the needed scopes:

The token does not have the scopes needed. Tokens need `read:packages` and `delete:packages`. The scopes found were none.

My GitHub App has packages:write permission and is installed on the org with access to all repositories. I didn't find a packages:delete option for GitHub Apps.

I'm not 100% sure, if I just used the action wrong or my App has wrong settings. But if it is not possible to have wildcard image names with GitHub App tokens, perhaps the README can be updated.

As soon as I switched to using a classical PAT, the workflow stated to work as expected.

Hmm, it sounds like we're assuming you're using $GITHUB_TOKEN when you're acually fine. Perhaps we should add a test to see whether requests like this will work or not, or find another way to differentiate the two token types.

As for the token permissions, I think these exist for PATs, but they may not do for app tokens? bilde

@makkes
Copy link

makkes commented Aug 6, 2024

keep-at-least has apparently been removed in v3 but it's not documented in the migration guide.

@sondrelg
Copy link
Member Author

sondrelg commented Aug 6, 2024

keep-at-least has apparently been removed in v3 but it's not documented in the migration guide.

It was renamed to keep-n-most-recent: https://github.com/snok/container-retention-policy/blob/main/action.yaml#L30. Sorry, you're right, I missed that in the release note. I'll add it now 👍

@simonjur
Copy link

simonjur commented Dec 4, 2024

in this code example in root reame.md:

skip-shas: ${{ steps.multi-arch-digests.outputs.multi-arch-shas }}

it should be

skip-shas: ${{ steps.multi-arch-shas.outputs.multi-arch-digests }}

names here were swapped multi-arch-shas with multi-arch-digests

no?

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

No branches or pull requests

6 participants