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

[FIX] Don't add docker.io prefix to ECR images #69

Merged

Conversation

der-eismann
Copy link
Contributor

@der-eismann der-eismann commented Aug 8, 2022

Description

The action assumes that if image.split("/").length == 2, the image must be from Docker hub instead of e.g. quay.io, where the value would be 3 (for example for quay.io/foreman/foreman:3.2-stable). But for AWS ECR (e.g. 123456789101.dkr.ecr.eu-west-1.amazonaws.com/example-project:master) the value is also 2, so it adds the prefix even though it is not needed which breaks the action.

Related Issue(s)

#68

Checklist

  • This PR includes a documentation change
  • This PR does not need a documentation change

  • This PR includes test changes
  • This PR's changes are already tested

  • This change is not user-facing
  • This change is a patch change
  • This change is a minor change
  • This change is a major (breaking) change

Changes made

  • Don't add docker.io prefix to ECR images pulled from Docker daemon

@github-actions github-actions bot added the CRDA Scan Pending CRDA scan waiting for approval label Aug 8, 2022
@der-eismann der-eismann force-pushed the dont-add-prefix-to-ecr-images branch from 6dc9b1e to 1c55984 Compare August 8, 2022 16:03
@der-eismann
Copy link
Contributor Author

I can confirm that this branch fixes our problems with the breaking builds. It would still be nice to disable the docker stuff altogether, but this is more important now.

@der-eismann
Copy link
Contributor Author

@divyansh42 Can you provide any feedback on this PR?

@der-eismann
Copy link
Contributor Author

@divyansh42 can you please get this in as well?

Copy link
Member

@divyansh42 divyansh42 left a comment

Choose a reason for hiding this comment

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

Thanks @der-eismann for your PR!

@divyansh42 divyansh42 added the CRDA Scan Approved CRDA scan approved by a collaborator label Feb 15, 2023
@github-actions github-actions bot added CRDA Scan Passed CRDA found no vulnerabilities and removed CRDA Scan Pending CRDA scan waiting for approval labels Feb 15, 2023
@divyansh42
Copy link
Member

@der-eismann could you please force push again? So that we can run the checks.

@github-actions github-actions bot added CRDA Scan Pending CRDA scan waiting for approval and removed CRDA Scan Approved CRDA scan approved by a collaborator CRDA Scan Passed CRDA found no vulnerabilities labels Feb 15, 2023
@der-eismann
Copy link
Contributor Author

I think you need to approve the workflows first before they run

@divyansh42
Copy link
Member

I think you need to approve the workflows first before they run

For some reason, earlier I was not able to see that option.
Now it is there. approved the workflow run.

@divyansh42 divyansh42 merged commit 7c03c8a into redhat-actions:main Feb 15, 2023
@der-eismann der-eismann deleted the dont-add-prefix-to-ecr-images branch February 15, 2023 12:27
@der-eismann
Copy link
Contributor Author

Thank you for merging!

@divyansh42
Copy link
Member

@der-eismann I have updated the tags, changes should reflect in v2, v2.7 and v2.7.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CRDA Scan Pending CRDA scan waiting for approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants