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.

Sign up for GitHub

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

Jump to bottom

Report all certificate errors instead of stopping at the first. (#941) #964

Merged
merged 2 commits into from
Jan 24, 2025
Merged

Report all certificate errors instead of stopping at the first. (#941) #964

merged 2 commits into from
Jan 24, 2025

Conversation

tristanmorgan
Copy link
Member

changed the loading of certificates process to report all errors found.

2025/01/23 22:44:34 [ERROR] cert: Failed to load certificates. 4 errors occurred:
	* cert: cannot load certificate nomad.service.consul-key.pem
	* cert: invalid certificate fabio.service.consul-cert.pem. tls: failed to find any PEM data in key input
	* cert: invalid certificate fabio.service.consul-key.pem. tls: failed to find any PEM data in key input
	* cert: cannot load certificate consul.service.consul-cert.pem

These errors reported were a result of deleting the nomad-cert, consul-key and uploading garbage to fabio-key.

@tristanmorgan tristanmorgan requested a review from a team January 23, 2025 22:50
dcarbone
dcarbone requested changes Jan 23, 2025
View reviewed changes
cert/load.go Outdated Show resolved Hide resolved
@tristanmorgan tristanmorgan self-assigned this Jan 23, 2025
@dcarbone dcarbone self-requested a review January 24, 2025 00:18
dcarbone
dcarbone approved these changes Jan 24, 2025
View reviewed changes
Copy link
Contributor

@dcarbone dcarbone left a comment

Choose a reason for hiding this comment

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

lgtm, @nathanejohnson or @aaronhurt?

aaronhurt
aaronhurt approved these changes Jan 24, 2025
View reviewed changes
Copy link
Member

@aaronhurt aaronhurt left a comment

Choose a reason for hiding this comment

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

lgtm as well, nice work

@tristanmorgan tristanmorgan merged commit 1928230 into fabiolb:master Jan 24, 2025
1 check passed
@tristanmorgan tristanmorgan deleted the report_all_certs branch January 24, 2025 05:36
@tristanmorgan tristanmorgan mentioned this pull request Jan 24, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronhurt aaronhurt aaronhurt approved these changes

@dcarbone dcarbone dcarbone approved these changes

Assignees

@tristanmorgan tristanmorgan

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tristanmorgan @dcarbone @aaronhurt