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

Ignore malformed secrets when creating keychain #1834

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lcarva
Copy link
Contributor

@lcarva lcarva commented Nov 1, 2023

This changes the behavior of the NewFromPullSecrets function to ignore secrets that contain malformed data. This allows processing of the other secrets which may be sufficient for authentication.

Resolves #1833.

@lcarva
Copy link
Contributor Author

lcarva commented Nov 3, 2023

@imjasonh , @jonjohnsonjr wdyt?

Copy link

github-actions bot commented Feb 2, 2024

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

@lcarva
Copy link
Contributor Author

lcarva commented Feb 2, 2024

It would be great to have a review here 🙏

@@ -186,7 +187,8 @@ func NewFromPullSecrets(ctx context.Context, secrets []corev1.Secret) (authn.Key
}
parsed, err := url.Parse(value)
if err != nil {
return nil, fmt.Errorf("Entry %q in dockercfg invalid (%w)", value, err)
logs.Warn.Printf("entry %q in dockercfg secret %s/%s invalid (%s); ignoring", secret.Namespace, secret.Name, value, err)
continue

Choose a reason for hiding this comment

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

Instead of continuing.
Can we throw an error if none of the secrets work? May be thats a failure mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

This changes the behavior of the NewFromPullSecrets function to ignore
secrets that contain malformed data. This allows processing of the other
secrets which may be sufficient for authentication.

Resolves google#1833

Signed-off-by: Luiz Carvalho <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Feb 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.67%. Comparing base (8dadbe7) to head (7a717a1).
Report is 20 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1834   +/-   ##
=======================================
  Coverage   71.67%   71.67%           
=======================================
  Files         123      123           
  Lines        9935     9935           
=======================================
  Hits         7121     7121           
  Misses       2115     2115           
  Partials      699      699           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chitrangpatel
Copy link

/approve

Copy link

@chitrangpatel chitrangpatel left a comment

Choose a reason for hiding this comment

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

Thanks for these changes!

Copy link

github-actions bot commented May 3, 2024

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

Copy link

github-actions bot commented Aug 2, 2024

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

Copy link

github-actions bot commented Nov 1, 2024

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

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.

ggcr: Ignore malformed secrets
3 participants