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 CC not restarted when API secret changes #9616

Merged
merged 7 commits into from Feb 2, 2024

Conversation

fvaleri
Copy link
Contributor

@fvaleri fvaleri commented Jan 30, 2024

The user may need to create new CC API credentials because they may have been compromised. This can be done by simply deleting the secret containing CC API credentials, which is then recreated by the CO. The problem is that CC is not restarted, which leads to the following Rebalance error:

2024-01-29 17:44:08 ERROR KafkaRebalanceAssemblyOperator:483 - Reconciliation #64(kafkarebalance-watch) KafkaRebalance(test/my-rebalance): Status updated to [NotReady] due to error: Unexpected status code 401 for request to my-cluster-cruise-control.test.svc:9090/kafkacruisecontrol/rebalance?json=true&dryrun=true&verbose=true&skip_hard_goal_check=false&rebalance_disk=false

To fix this issue, I'm adding the API secret hash as CC annotation, so that any change will trigger a CC pod restart.

The user may need to create new CC API credentials because they may have been compromised.
This can be done by simply deleting the secret containing CC API credentials, which is then recreated by the CO.
The problem is that CC is not restarted, which leads to the following Rebalance error:

```sh
2024-01-29 17:44:08 ERROR KafkaRebalanceAssemblyOperator:483 - Reconciliation strimzi#64(kafkarebalance-watch) KafkaRebalance(test/my-rebalance): Status updated to [NotReady] due to error: Unexpected status code 401 for request to my-cluster-cruise-control.test.svc:9090/kafkacruisecontrol/rebalance?json=true&dryrun=true&verbose=true&skip_hard_goal_check=false&rebalance_disk=false
```

To fix this issue, I'm adding the API secret hash as CC annotation, so that any change will trigger a CC pod restart.

Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri fvaleri added this to the 0.40.0 milestone Jan 30, 2024
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri
Copy link
Contributor Author

fvaleri commented Jan 30, 2024

@scholzj I'm now hashing all passwords in the API secret. Test updated.

Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri fvaleri requested review from kyguy and scholzj January 31, 2024 11:16
Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

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

LGTM. Just a couple of nits.

Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
Copy link
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

One nit. LGTM otherwise.

@scholzj
Copy link
Member

scholzj commented Feb 1, 2024

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@kyguy kyguy left a comment

Choose a reason for hiding this comment

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

Awesome work, thanks Fede!

Signed-off-by: Federico Valeri <fedevaleri@gmail.com>
@fvaleri
Copy link
Contributor Author

fvaleri commented Feb 1, 2024

@scholzj I tried SecurityST#testAutoRenewClientsCaCertsTriggeredByAnno multiple times with the latest implementation on my machine and it seems to work fine.

@scholzj
Copy link
Member

scholzj commented Feb 1, 2024

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj scholzj merged commit a4953e0 into strimzi:main Feb 2, 2024
21 checks passed
@fvaleri fvaleri deleted the fix-cc-credentials-update branch February 2, 2024 07:15
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.

None yet

4 participants