Add support for custom CC API users #10117

kyguy · 2024-05-15T20:50:47Z

Type of change

Enhancement / new feature

Description

Based on this proposal, this PR allows advanced users the ability to create REST API users for the Cruise Control REST API. This would allow roles and permissions to be defined to allow advanced users and third-party applications to access the Cruise Control REST API without having to disable HTTP basic authentication.

For example, advanced users could define their custom API users in a secret called cruise-control-api-users-secret putting their API user credentials in the Jetty's HashLoginService's file format like this:

userOne: passwordOne, USER
userTwo: passwordTwo, VIEWER

Create the secret:

kubectl create secret generic cruise-control-api-users-secret  --from-file=key=cruise-control-auth.txt

Then update their Kafka custom resource like this:

apiVersion: kafka.strimzi.io/v1beta2
kind: Kafka
metadata:
  ...
spec:
  cruiseControl:
    apiUsers:
      type: hashLoginService (1)
      valueFrom: (2)
        secretKeyRef:
          name: cruise-control-api-users-secret
          key: key
     ...

(1) A type field is added here to describe the format of the data and for configuration flexibility in the future. This gives us the option to add different data format types in the future. In this example we use Jetty's HashLoginService format since that is the format which Cruise Control uses for its API user configuration.

(2) The valueFrom construct allows us to add more sources in the future if needed. This is also a pattern used in other Strimzi APIs already, for example Strimzi's logging configuration, password configuration in KafkaUser resources, metrics configuration, and more.

For more information, checkout the proposal.

Checklist

Please go through this checklist and make sure all applicable tasks have been done

Write tests
Make sure all tests pass
Update documentation
Check RBAC rights for Kubernetes / OpenShift roles
Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
Reference relevant issue(s) and close them after merging
Update CHANGELOG.md
Supply screenshots for visual changes, such as Grafana dashboards

kyguy · 2024-06-05T22:14:10Z

/packit test --labels sanity

Frawless · 2024-06-06T04:53:33Z

I just wanted to check something with Packit. The failure is not connected to this PR but to Packit/TF config, you can ignore the failed TF checks, thanks.

fvaleri

Hi @kyguy, nice work. It is a useful feature that can be leveraged by UI tools that want to integrate with Strimzi, so it deserves a mention in the release notes. Overall LGTM, but I found a minor issue, and left some improvement suggestions.

When the TO starts it fails to load the username and password for the admin user. You changed the keys in CruiseControlApiProperties, and they don't match with the TO config class. Also, note that these keys are public API (see standalone deployment doc).

To fix the issue, you have to revert this change. Alternatively you can update the TO config class, documentation, and provide a deprecation period. I'm fine with both.

Exception in thread "main" java.io.UncheckedIOException: File not found: /etc/eto-cc-api/topic-operator.apiAdminName
	at io.strimzi.operator.topic.ReplicasChangeHandler.getFileContent(ReplicasChangeHandler.java:169)
	at io.strimzi.operator.topic.ReplicasChangeHandler.<init>(ReplicasChangeHandler.java:60)
	at io.strimzi.operator.topic.TopicOperatorMain.<init>(TopicOperatorMain.java:70)
	at io.strimzi.operator.topic.TopicOperatorMain.operator(TopicOperatorMain.java:161)
	at io.strimzi.operator.topic.TopicOperatorMain.main(TopicOperatorMain.java:156)
Caused by: java.nio.file.NoSuchFileException: /etc/eto-cc-api/topic-operator.apiAdminName
	at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:92)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:106)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
	at java.base/sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:218)
	at java.base/java.nio.file.Files.newByteChannel(Files.java:380)
	at java.base/java.nio.file.Files.newByteChannel(Files.java:432)
	at java.base/java.nio.file.Files.readAllBytes(Files.java:3288)
	at io.strimzi.operator.topic.ReplicasChangeHandler.getFileContent(ReplicasChangeHandler.java:167)
	... 4 more

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

api/src/main/java/io/strimzi/api/kafka/model/kafka/cruisecontrol/ApiUsers.java

fvaleri · 2024-06-07T13:34:19Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ /**
+ * Represents a single API user entry including name, password, and role.
+ */
+ public record Entry(String username, String password, Role role) { }


UserEntry maybe?

scholzj · 2024-06-08T16:30:17Z

api/src/main/java/io/strimzi/api/kafka/model/kafka/cruisecontrol/ApiUsers.java

+@JsonInclude(JsonInclude.Include.NON_NULL)
+@JsonPropertyOrder({"type", "valueFrom"})
+@EqualsAndHashCode
+public class ApiUsers implements UnknownPropertyPreserving {


Maybe you should call it something like HashLoginServiceApiUsers?

scholzj · 2024-06-08T17:24:15Z

api/src/main/java/io/strimzi/api/kafka/model/kafka/cruisecontrol/ApiUsers.java

+
+ @Description("Must be `" + TYPE_HASH_LOGIN_SERVICE + "`")
+ @JsonInclude(JsonInclude.Include.NON_NULL)
+ public String getType() {


This should be required? You should also improve the description ... Type of the Cruise Control API Users. Supported values are: ... or something like that.

api/src/main/java/io/strimzi/api/kafka/model/kafka/cruisecontrol/ApiUsers.java

scholzj · 2024-06-08T17:28:00Z

api/src/main/java/io/strimzi/api/kafka/model/kafka/cruisecontrol/CruiseControlSpec.java

@@ -109,6 +110,16 @@ public void setConfig(Map<String, Object> config) {
 this.config = config;
 }

+ @Description("The Cruise Control `ApiUsers` configuration")


Maybe you should explain what this is? MAybe something like this?

Suggested change

@Description("The Cruise Control `ApiUsers` configuration")

@Description("Configuration of the Cruise Control REST API users.")

scholzj · 2024-06-08T17:30:48Z

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CruiseControl.java

+ || apiUsers.getValueFrom().getSecretKeyRef() == null
+ || apiUsers.getValueFrom().getSecretKeyRef().getName() == null
+ || apiUsers.getValueFrom().getSecretKeyRef().getKey() == null) {
+ throw new InvalidResourceException("Resource requests custom ApiUsers config but doesn't specify the secret name and key");


I'm not sure if anyone else would understand what this means. Maybe you could say something like The configuring of Cruise Control REST API users in spec.cruiseControl.apiUsers is invalid..

Also, aren't the users always enabled and the only thing that changes is whether we add user-defined users as well or not?

Also, aren't the users always enabled and the only thing that changes is whether we add user-defined users as well or not?

Yes, just updated the method names to hopefully make this more clear

scholzj · 2024-06-08T17:47:02Z

...tor/src/main/java/io/strimzi/operator/cluster/operator/assembly/CruiseControlReconciler.java

+ Integer centralizedApiSecretIndex = 0;
+ Integer userManagedApiSecretIndex;
+ Integer topicOperatorManagedApiSecretIndex;
+
+ List<Future<Secret>> futures = new ArrayList<>();
+ // Centralized API secret
+ futures.add(secretOperator.getAsync(reconciliation.namespace(), CruiseControlResources.apiSecretName(reconciliation.name())));
+
+ if (cruiseControl.apiUsersEnabled()) {
+ // User-managed API secret
+ futures.add(secretOperator.getAsync(reconciliation.namespace(), cruiseControl.getUserManagedApiSecretName()));
+ userManagedApiSecretIndex = futures.size() - 1;
+ } else {
+ userManagedApiSecretIndex = null;
+ }
+
 if (isTopicOperatorEnabled) {
- return Future.join(
- secretOperator.getAsync(reconciliation.namespace(), CruiseControlResources.apiSecretName(reconciliation.name())),
- secretOperator.getAsync(reconciliation.namespace(), KafkaResources.entityTopicOperatorCcApiSecretName(reconciliation.name()))
- ).compose(
+ // Topic operator-managed API secret
+ futures.add(secretOperator.getAsync(reconciliation.namespace(), KafkaResources.entityTopicOperatorCcApiSecretName(reconciliation.name())));
+ topicOperatorManagedApiSecretIndex = futures.size() - 1;
+ } else {
+ topicOperatorManagedApiSecretIndex = null;
+ }


This seems weird. Maybe instead of the indexes, you can just use succeeded futures with null result. And if you name them instead of using list, you save the list creation.

You are right, just cleaned it up now

scholzj · 2024-06-08T17:48:16Z

...tor/src/main/java/io/strimzi/operator/cluster/operator/assembly/CruiseControlReconciler.java

+ Map<String, ApiCredentials.Entry> apiCredentials = new HashMap<>();
+ apiCredentials.putAll(ApiCredentials.generateCoManagedApiCredentials(passwordGenerator, oldCentralizedApiSecret));
+ apiCredentials.putAll(ApiCredentials.generateUserManagedApiCredentials(userManagedApiSecret, cruiseControl.getUserManagedApiSecretKey()));
+ apiCredentials.putAll(ApiCredentials.generateToManagedApiCredentials(topicOperatorManagedApiSecret));
+
+ Map<String, String> mapWithApiCredentials = ApiCredentials.generateMapWithApiCredentials(apiCredentials);
+ Secret newCentralizedApiUsersSecret = cruiseControl.generateApiSecret(mapWithApiCredentials);


As I commented on the utility class ... this should be maybe wrapped into one public method there?

...src/test/java/io/strimzi/operator/cluster/operator/assembly/CruiseControlReconcilerTest.java

scholzj · 2024-06-08T17:54:28Z

documentation/modules/cruise-control/proc-configuring-deploying-cruise-control.adoc

+ apiUsers:
+ type: hashloginservice <11>
+ valueFrom: <12>
+ secretKeyRef:
+ name: cruise-control-api-users-secret
+ key: key


Do we want this included here if it is for advanced users?

Yeah, once merged, we can simply publish a small blog post showing how to use it with some Kafka web console (maybe referencing the old post about this).

Removed this in the procedure doc here but left the documentation in the API docs

Yeah, once merged, we can simply publish a small blog post showing how to use it with some Kafka web console (maybe referencing the old post about this).

Sounds reasonable to me, I also plan on updating the "Hacking Strimzi for Cruise Control UI" post with this information as well

scholzj · 2024-06-08T18:00:58Z

/azp run regression

azure-pipelines · 2024-06-08T18:01:10Z

Azure Pipelines successfully started running 1 pipeline(s).

ppatierno · 2024-06-11T09:42:49Z

api/src/main/java/io/strimzi/api/kafka/model/kafka/cruisecontrol/ApiUsers.java

+@JsonInclude(JsonInclude.Include.NON_NULL)
+@JsonPropertyOrder({"type", "valueFrom"})
+@EqualsAndHashCode
+public class ApiUsers implements UnknownPropertyPreserving {


ppatierno · 2024-06-11T09:51:09Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ * authenticating to Cruise Control's REST API.
+ */
+public class ApiCredentials {
+ // Regex to match a slightly altered version of Jetty's HashLoginService's file format: username: password, rolename


Yeah, you are mentioning we are using Jetty HashLoginService even in the PR description (but not a slightly altered version).

ppatierno · 2024-06-11T10:01:15Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ } else {
+ throw new InvalidConfigurationException("Invalid configuration provided: " + "\"" + line + "\". " +
+ "Cruise Control API credentials config must follow " +
+ "HashLoginService's file format: username: password [,rolename ...]");


Suggested change

"HashLoginService's file format: username: password [,rolename ...]");

"HashLoginService's file format 'username: password [,rolename ...]'");

I just suggested to put the format between quotes to make it clearer where it starts and ends between the sentence.

ppatierno · 2024-06-11T10:03:57Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ if (FORBIDDEN_USERNAMES.contains(username)) {
+ throw new InvalidConfigurationException("The following usernames for Cruise Control API are forbidden: " + FORBIDDEN_USERNAMES
+ + " User provided Cruise Control API credentials contain illegal username: " + username);
+


unnecessary blank line

fvaleri

LGTM. Thanks.

scholzj · 2024-06-12T08:36:38Z

CHANGELOG.md

+* Location of Topic Operator API credentials for Cruise Control has changed from
+ `/etc/eto-cc-api/topic-operator.apiAdminUsername` -> `/etc/eto-cc-api/topic-operator.username` for username
+ `/etc/eto-cc-api/topic-operator.apiAdminPassword` -> `/etc/eto-cc-api/topic-operator.password` for password.


Why are we changing this? What is the motivation or reason?

To standardize the username and password file naming for all CC API users in a way that better describes each API user's purpose.

For example in this PR:

The file that holds the password for the RebalanceOperator CC API user was changed:
cruise-control.apiAdminPassword -> rebalance-operator.password

The file that holds the password for the API user that checks if the CC API is up and running was changed
cruise-control.apiUserPassword -> healthcheck.password.

Lastly to follow the pattern:

The file that holds the username for the TopicOperator CC API user was changed:

topic-operator.apiAdminUsername -> topic-operator.username

The file that holds the password for the TopicOperator CC API password was changed:

topic-operator.apiAdminPassword -> topic-operator.password

But this change affects users, so is it really worth it?

But this change affects users, so is it really worth it?

People using the standalone TO with Cruise Control would need to be aware of this filename change and update their TO deployments accordingly. Not a huge price to pay for standardized naming but I understand how the change could be annoying.

If you feel strongly about it, I'll revert the change

I would not feel strongly about it if it had some good reason. But it seems it is just a change for a sake of change. What are you standardizing? There is only one TO with these names. They don't relate to anything else, or?

@fvaleri But you would need to do that in the TO I guess and check if the files are or aren't there and use an alternative one? The CC feature in TO is relatively new, so I'm not sure it is worth the deprecation and removal later if we want to use the new names long term.

@ppatierno Any thoughts on this?

@kyguy The TO Secret is IIRC owned by the TO. So why can't it use its names? The names of the other secret used by the CO matter less as they are not user-facing.

The CC feature in TO is relatively new, so I'm not sure it is worth the deprecation and removal later if we want to use the new names long term.

I was just proposing an alternative, but I also feel that the motivation for this change is not strong enough, so my vote is on reverting the change.

I am for having better and consistent naming so I would like this change but ...

People using the standalone TO with Cruise Control would need to be aware of this filename change and update their TO deployments accordingly

How many users are really using the TO with Cruise Control in standalone mode which is a pretty new thing. AFAIU it's going to be just a matter of describing the "breaking" change in the CHANGELOG.
Of course, deprecating and remove could be the way for this kind of things but again, I don't think many people are using it.

The TO Secret is IIRC owned by the TO. So why can't it use its names?

It is, and it can use the existing names no problem.

The names of the other secret used by the CO matter less as they are not user-facing.

True. While I was updating the key names in to CO API secret I figured I would update the key names in the TO API secret while I was in there to match the format. However, it does not seem worth it since the key names of the TO API secret are user facing. TBH I don't want to complicate things or delay this PR any longer. If it is ok with everyone, I will revert the TO API secret key names to how they were

scholzj · 2024-06-13T07:33:06Z

cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CruiseControl.java

@@ -121,6 +115,9 @@ public class CruiseControl extends AbstractModel implements SupportsMetrics, Sup

 private boolean sslEnabled;
 private boolean authEnabled;
+ private boolean userManagedApiUsersEnabled;


Do you really need this boolean? You can probably check it through the userManagedApiSecretName field for example and change the isUserManagedApiUsersConfigEnabled method to something like validateApiUsersConfig, or?

You are right, I should be able to safely removed now. Just removed

scholzj · 2024-06-13T07:36:42Z

...tor/src/main/java/io/strimzi/operator/cluster/operator/assembly/CruiseControlReconciler.java

- secretOperator.getAsync(reconciliation.namespace(), CruiseControlResources.apiSecretName(reconciliation.name())),
- secretOperator.getAsync(reconciliation.namespace(), KafkaResources.entityTopicOperatorCcApiSecretName(reconciliation.name()))
- ).compose(
+ Future<Secret> centralizedApiSecretFuture = secretOperator.getAsync(reconciliation.namespace(), CruiseControlResources.apiSecretName(reconciliation.name()));


What is centralizedApiSecretFuture? I don't think the centralized has a clear meaning. Maybe ccApiSecretFuture?

scholzj · 2024-06-13T07:38:10Z

...tor/src/main/java/io/strimzi/operator/cluster/operator/assembly/CruiseControlReconciler.java

+ Secret userManagedApiSecret = compositeFuture.resultAt(1);
+ Secret topicOperatorManagedApiSecret = compositeFuture.resultAt(2);
+
+ Secret newCentralizedApiUsersSecret = ApiCredentials.generateApiSecret(cruiseControl, passwordGenerator,


Same as above, I would call it ccApiSecret or something like that. In the model class, you also don't call it geenrateCentralizedApiSecret either.

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

scholzj · 2024-06-13T07:47:15Z

/azp run regression

azure-pipelines · 2024-06-13T07:47:27Z

Azure Pipelines successfully started running 1 pipeline(s).

scholzj

I think the structure looks good. I left some more nits about some details such as access modifiers etc.

scholzj · 2024-06-17T21:31:37Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ private String userManagedApiSecretName;
+ private String userManagedApiSecretKey;


I guess you can make it final and set it to null if user didn't configured it?

scholzj · 2024-06-17T21:32:21Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ this.ownerReference = ownerReference;
+
+ if (validateApiUsersConfig(specSection)) {
+ HashLoginServiceApiUsers apiUsers = specSection.getApiUsers();


Maybe you can use directly specSection.getApiUsers() on the two lines below?

scholzj · 2024-06-17T21:32:36Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ * For more information checkout the upstream Cruise Control Wiki here:
+ * <a href="https://github.com/linkedin/cruise-control/wiki/Security#authorization">Cruise Control Security</a>
+ */
+ public enum Role {


I would put it at the end of the file.

scholzj · 2024-06-17T21:32:52Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ /**
+ * Represents a single API user entry including name, password, and role.
+ */
+ public record UserEntry(String username, String password, Role role) { }


Same as above, I would put it at the end of the file.

scholzj · 2024-06-17T21:33:52Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ *
+ * @return map of API credential entries containing username, password, and role.
+ */
+ public static Map<String, UserEntry> parseEntriesFromString(String config) {


I guess this can be private now or /* test */?

scholzj · 2024-06-17T21:35:17Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ *
+ * @return Map of API user entries containing Strimzi-managed API user credentials
+ */
+ public static Map<String, UserEntry> generateCoManagedApiCredentials(PasswordGenerator passwordGenerator, Secret secret) {


I guess this can be private now or /* test */?

scholzj · 2024-06-17T21:35:36Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ *
+ * @return Map containing Cruise Control API auth credentials
+ */
+ public static Map<String, String> generateMapWithApiCredentials(Map<String, UserEntry> entries) {


I guess this can be private?

Using this method in the MockCruiseControl class, so I prefixed the method with /* test */

scholzj · 2024-06-17T21:37:04Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ apiCredentials.putAll(generateCoManagedApiCredentials(passwordGenerator, oldCruiseControlApiSecret));
+ apiCredentials.putAll(generateUserManagedApiCredentials(userManagedApiSecret, userManagedApiSecretKey));
+ apiCredentials.putAll(generateToManagedApiCredentials(topicOperatorManagedApiSecret));


Would it make sense to pass the three parameters into generateMapWithApiCredentials instead of creating a map just to pass them there?

The map as a whole is also used for the value of the AUTH_FILE_KEY entry in the map that is created in generateMapWithApiCredentials (There is a map turned to a String for a value in the map of generateMapWithApiCredentials). I could move the map creation there, but I still think it would be more simple to leave it where is

I could move the map creation there

I think that would be much better, because it would be much clearer what the parameters are and you don't really save anything by doing it outside.

Just tried moving it over, you are right it does look neater

scholzj · 2024-06-17T21:40:06Z

...perator/src/test/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentialTest.java

Should you add also some tests for the validation and creation of the object and the Secret?

scholzj · 2024-06-17T21:41:19Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ */
+ public Secret generateApiSecret(PasswordGenerator passwordGenerator,
+ Secret oldCruiseControlApiSecret,
+ Secret userManagedApiSecret,


Maybe I missed it, but do you have some validation what if the user configured to use a Secret but that Secret is missing? We should throw there some InvalidResourceException or something.

Nice catch, added

scholzj

Few more nits. Also, if you rebase it we should be able to run the tests.

scholzj · 2024-06-20T18:20:33Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ /**
+ * @return Returns user-managed API credentials secret key
+ */
+ /* test */ public String getUserManagedApiSecretKey() {


It is either public or /* test */. Not both. The /* test */ is used to indicate that methods with default access modifiers have the default one because of tests. But if it is public, it makes no sense really.

Suggested change

/* test */ public String getUserManagedApiSecretKey() {

public String getUserManagedApiSecretKey() {

scholzj · 2024-06-20T18:22:41Z

...r-operator/src/main/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentials.java

+ *
+ * @return map of API credential entries containing username, password, and role.
+ */
+ /* test */ public static Map<String, UserEntry> parseEntriesFromString(String config) {


Same as above ... and there are more below. If you need to use it in tests, that is fine. But the test using the non-public / low-level methods like these should be in the same package. So you normally need only instead of making it public:

Suggested change

/* test */ public static Map<String, UserEntry> parseEntriesFromString(String config) {

/* test */ static Map<String, UserEntry> parseEntriesFromString(String config) {

scholzj · 2024-06-20T18:26:25Z

...test/java/io/strimzi/operator/cluster/operator/resource/cruisecontrol/MockCruiseControl.java


+ private static Map<String, String> apiSecretData = ApiCredentials.generateMapWithApiCredentials(ApiCredentials.generateCoManagedApiCredentials(new PasswordGenerator(16), null));


Why not have this hardcoded? It does not seem like this thing is somehow dynamically changing

scholzj · 2024-06-20T18:28:53Z

...perator/src/test/java/io/strimzi/operator/cluster/model/cruisecontrol/ApiCredentialTest.java

+ private static Secret createSecret(Map<String, String> data) {
+ return new SecretBuilder()
+ .withNewMetadata()
+ .withName(SECRET_NAME)


If you indent it, it will have better readability 😉

scholzj · 2024-06-21T00:23:02Z

/azp run regression

azure-pipelines · 2024-06-21T00:23:13Z

Azure Pipelines successfully started running 1 pipeline(s).

scholzj · 2024-06-21T08:20:12Z

@kyguy The code looks good to me. But there seems to be some issue as all tests with CC failed in regression.

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

scholzj · 2024-08-05T13:47:56Z

@kyguy Could you please resolve the conflict?

kyguy · 2024-08-06T12:59:03Z

@scholzj COnflict resolved and now have all approvals, could you execute regression tests one more time

scholzj · 2024-08-06T13:00:09Z

Yeah, I need to first merge #10415 that fixes some flaky tests.

scholzj · 2024-08-06T16:15:02Z

/azp run regression

azure-pipelines · 2024-08-06T16:15:14Z

Azure Pipelines successfully started running 1 pipeline(s).

scholzj · 2024-08-06T21:08:43Z

Thanks for the PR @kyguy

kyguy force-pushed the support-cc-api-users branch 3 times, most recently from 7a56e59 to ab23356 Compare June 5, 2024 15:43

kyguy marked this pull request as ready for review June 5, 2024 18:14

kyguy added this to the 0.42.0 milestone Jun 5, 2024

kyguy requested review from fvaleri, ppatierno, PaulRMellor, scholzj and ShubhamRwt June 5, 2024 18:15

fvaleri suggested changes Jun 7, 2024

View reviewed changes

scholzj reviewed Jun 8, 2024

View reviewed changes

ppatierno reviewed Jun 11, 2024

View reviewed changes

fvaleri approved these changes Jun 12, 2024

View reviewed changes

scholzj reviewed Jun 12, 2024

View reviewed changes

scholzj reviewed Jun 13, 2024

View reviewed changes

scholzj reviewed Jun 17, 2024

View reviewed changes

scholzj added the cluster-balancing label Jun 19, 2024

scholzj reviewed Jun 20, 2024

View reviewed changes

kyguy force-pushed the support-cc-api-users branch from 4e15dc7 to 9354146 Compare June 20, 2024 20:26

ShubhamRwt approved these changes Jun 24, 2024

View reviewed changes

kyguy added 21 commits August 5, 2024 09:39

Addressing feedback - js, fv

df4923a

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Refactoring ApiCredentials based on feedback - js

f4747b7

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing more feedback -js

43a849d

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing more feedback - js

c3158af

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Fix CruiseControlReconcilerTest.java test

62cd728

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Fix other tests

11cf735

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Fix system tests

4793b1c

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing feedback - js

ff15d1f

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Revert doc change

df84a0d

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing feedback - pp

ce1ffcb

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing feedback - pm

947592d

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Refactor based on feedback - pp

ba02854

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing more feedback -pp

3569a91

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing remaining comments - pp

8ca8e65

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing comments - pp

1a60701

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Refactor logic out of Strimzi API

c0dba89

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing most comments - pp, js

b4a4d94

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing more feedback - js

26de0d2

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Reduce HashMap creation

015e1c2

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Fix unit tests

f232e44

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Add punctuation to exception comments

ebe514d

Signed-off-by: Kyle Liberti <kliberti@redhat.com>

kyguy force-pushed the support-cc-api-users branch 2 times, most recently from cbfb9cb to ebe514d Compare August 5, 2024 14:05

scholzj merged commit 684db92 into strimzi:main Aug 6, 2024
21 checks passed

	@Description("The Cruise Control `ApiUsers` configuration")
	@Description("Configuration of the Cruise Control REST API users.")

	"HashLoginService's file format: username: password [,rolename ...]");
	"HashLoginService's file format 'username: password [,rolename ...]'");

		private String userManagedApiSecretName;
		private String userManagedApiSecretKey;

	/* test */ public String getUserManagedApiSecretKey() {
	public String getUserManagedApiSecretKey() {

	/* test */ public static Map<String, UserEntry> parseEntriesFromString(String config) {
	/* test */ static Map<String, UserEntry> parseEntriesFromString(String config) {


		private static Map<String, String> apiSecretData = ApiCredentials.generateMapWithApiCredentials(ApiCredentials.generateCoManagedApiCredentials(new PasswordGenerator(16), null));

Add support for custom CC API users #10117

Add support for custom CC API users #10117

Conversation

kyguy commented May 15, 2024 • edited Loading

Type of change

Description

Checklist

kyguy commented Jun 5, 2024

Frawless commented Jun 6, 2024

fvaleri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scholzj commented Jun 8, 2024

azure-pipelines bot commented Jun 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fvaleri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyguy Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fvaleri Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyguy Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scholzj commented Jun 13, 2024

azure-pipelines bot commented Jun 13, 2024

scholzj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyguy Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scholzj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scholzj commented Jun 21, 2024

azure-pipelines bot commented Jun 21, 2024

scholzj commented Jun 21, 2024

scholzj commented Aug 5, 2024

kyguy commented May 15, 2024 •

edited

Loading

kyguy Jun 12, 2024 •

edited

Loading

fvaleri Jun 13, 2024 •

edited

Loading

kyguy Jun 14, 2024 •

edited

Loading

kyguy Jun 20, 2024 •

edited

Loading