AthenZ / k8s-athenz-sia

Apache License 2.0

1 stars 7 forks source link

fix certreloader update logic #111

Closed t4niwa closed 5 months ago

t4niwa commented 5 months ago

Description

panic occurred because cert reloader attempted to update even though certificate retrieval failed

This PR adds the red box in the following diagram:

Type of change

[x] Bug fix
[ ] New feature
[ ] Refactoring (no functional changes, no api changes)
[ ] Non-code changes (update documentation, pipeline, etc.)

Flags

[ ] Breaks backward compatibility
[ ] Requires a documentation update
[ ] Has untestable code

Related issue/PR

Delete this section if there are no issues or pull requests that relate to this pull request.

Fixes #issue
Closes #PR

Checklist

[ ] Followed the guidelines in the CONTRIBUTING document
[ ] Added prefix [skip ci]/[ci skip]/[no ci]/[skip actions]/[actions skip] in the PR title if necessary
[ ] Tested and linted the code
[ ] Commented the code
[ ] Made corresponding changes to the documentation
[ ] Passed all pipeline checking

Checklist for maintainer

Use Squash and merge
Double-confirm the merge message has prefix [skip ci]/[ci skip]/[no ci]/[skip actions]/[actions skip]
Delete the branch after merge

mlajkim commented 5 months ago

[x] handle

if config.ProviderService == "" && config.CertFile != "" && config.KeyFile != "" {
go r.pollRefresh()
}

mlajkim commented 5 months ago

Probably do the similar changes as @t4niwa you've done in https://github.com/AthenZ/k8s-athenz-sia/pull/111/commits/0b89c129bade62f08f0c94c9dcd40a6146134c02

BUT, make sure not to use err = ... but instead use a different name not to override the errSomething := ...

[x] Do the task above
[x] Make a PR (or issue) that brings the reload before the roleCertProvisioning