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 build failures when Docker auth check fails #1191

Merged
merged 2 commits into from
Nov 12, 2024

Conversation

eager-signal
Copy link
Contributor

fixes #1172

@CLAassistant
Copy link

CLAassistant commented Sep 25, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

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

can you add a test?

@sdelamo sdelamo changed the base branch from 4.6.x to 4.7.x October 3, 2024 08:06
@eager-signal eager-signal force-pushed the gh-1172 branch 3 times, most recently from 58b542d to 8f4bccc Compare October 3, 2024 21:16
@graemerocher graemerocher requested a review from sdelamo October 3, 2024 21:18
@eager-signal
Copy link
Contributor Author

can you add a test?

@sdelamo done. I confirmed that the test fails without the change and passes afterward.

@sdelamo sdelamo requested a review from alvarosanchez October 4, 2024 13:28
@sdelamo sdelamo added the type: improvement A minor improvement to an existing feature label Oct 4, 2024
Comment on lines 4 to 5
assert log.text.contains("[WARNING] Failed to login to registry") : "Credentials check should be a soft failure"
assert log.text.contains(" Could not push image: denied") : "A push should always be attempted"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test could be revised to be confirm a successful push after the soft failure, if there is a GCR analog to phx.ocir.io/oraclelabs/micronaut-maven-plugin available.

@alvarosanchez
Copy link
Member

@eager-signal I'm testing this locally, and am getting the same error as in CI: DockerService#getAuthConfigFor is not even called (credential is null), so the check you added there is not having any effect.

Maybe you're logged in locally and you are getting different results?

@eager-signal
Copy link
Contributor Author

Maybe you're logged in locally and you are getting different results?

@alvarosanchez ah, this is possible. I will do some testing in a clean environment.

@eager-signal eager-signal force-pushed the gh-1172 branch 2 times, most recently from da60c25 to e911b57 Compare November 1, 2024 22:09
@eager-signal
Copy link
Contributor Author

Maybe you're logged in locally and you are getting different results?

@alvarosanchez thanks for the tip. I cleared my ~/.docker/config.json locally, and that was indeed the issue. (There was also a second issue—now that testcontainers 1.20.3 is out with its bug fixed, the soft failure was also gone when my credentials were present.)

I just pushed an update to the gh1172-deploy integration test with some invalid credentials, so now it should fail without the fix, and pass correctly with the fix. I also added an integration test for the bug’s package case.

@eager-signal
Copy link
Contributor Author

Hmm, the issue1172-package test passes for me locally with an empty ~/.docker/config.json—I wonder if there’s some default credentials for Docker Hub being discovered. In any case, I pushed a change to use a different registry, the GCR cache, as the image source.

@alvarosanchez alvarosanchez merged commit 072e9ee into micronaut-projects:4.7.x Nov 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement A minor improvement to an existing feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Build fails when Docker auth check fails, rather than proceeding with a warning
4 participants