-
Notifications
You must be signed in to change notification settings - Fork 15
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 'Docker Login' Step back to support required cases #365
Conversation
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.
@dmikusa-pivotal Could I get your eyes on this please before I cut a release, validate & roll out?
I had to change the conditional logic a bit to still support the old checks (e.g. no forks) and add the Dependabot actor check
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.
The logic seems good to me. You have the same logic as before &&
'd with is_not_dependabot
. So every case that was previously true will only be true if the actor is not dependabot. Every case that was previously false will still be false regardless of the actor. Anything submitted by dependabot will always be false. I think that's what we want.
Summary
Removing the Docker Login step completely from 'Create Package Test' action caused failures for some composite buildpacks where it is required. This patch adds the step back for non Paketo buildpacks and also adds a check which will not run the Login step if the actor is Dependabot, since it will not have the required credentials to succeed(recommended by Github Support).
Use Cases
The Docker Login step will continue to fail for Dependabot PRs in some composite buildpacks, re-running the action should supply the credentials and the check should then pass.
Checklist