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

Operator auth with serviceAccount and split mgmt and app realms #423

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ruromero
Copy link
Contributor

Fix #417

Depends on

Based on the broker version, the operator will authenticate using the serviceAccount or the username/password. The operator will also configure different realms for management and application users.

@ruromero ruromero marked this pull request as ready for review December 19, 2022 16:25
@ruromero ruromero marked this pull request as draft December 19, 2022 16:26
@ruromero ruromero marked this pull request as ready for review December 19, 2022 20:40
@ruromero ruromero changed the title Sa login Operator auth with serviceAccount and split mgmt and app realms Dec 19, 2022
@ruromero ruromero marked this pull request as draft December 20, 2022 20:39
@ruromero
Copy link
Contributor Author

The PR is missing the decision of whether to authenticate using the service account or the username/password

@ruromero
Copy link
Contributor Author

Thanks @gaohoward for the missing bits.

@gtully
Copy link
Contributor

gtully commented Dec 22, 2022

this is great :-)

I am wondering about the two realms, it would be ideal to be able to use the service account for broker to broker comms, like a federation connection, but that will use the "activemq" realm. Afaik, the security manager uses a single realm "activemq" by default. It is configurable, but there is only one.

with the security.properties approach - is there a merge option? That will bring its own potential problems too, but I think it may be necessary. ie: into the activemq realm we add the service account login module for use inter-broker and by management

the other thought, separate, is that we do this just for the jolokia agent and always configure that for the operator. In that way we are locking down the broker and we deprecate the deployment of the web console war.

@ruromero
Copy link
Contributor Author

ruromero commented Jan 4, 2023

this is great :-)

I am wondering about the two realms, it would be ideal to be able to use the service account for broker to broker comms, like a federation connection, but that will use the "activemq" realm. Afaik, the security manager uses a single realm "activemq" by default. It is configurable, but there is only one.

Isn't federation configured with a specific credentials? https://access.redhat.com/documentation/en-us/red_hat_amq/2020.q4/html/configuring_amq_broker/assembly-br-configuring-addresses-and-queues_configuring#proc-br-configuring-upstream-address-federation_configuring
I don't see how to use here the jaas configuration

with the security.properties approach - is there a merge option? That will bring its own potential problems too, but I think it may be necessary. ie: into the activemq realm we add the service account login module for use inter-broker and by management

Yes, this security.properties file adds an additional login.config [1] file that will be appended to the first one available on the default location [0]. This allows users to customize the activemq realm and also provide additional management configurations.

the other thought, separate, is that we do this just for the jolokia agent and always configure that for the operator. In that way we are locking down the broker and we deprecate the deployment of the web console war.

I think, the jolokia agent is the only one using the management realm.

@ruromero ruromero force-pushed the sa-login branch 3 times, most recently from a12f43b to 7122571 Compare January 10, 2023 15:22
@ruromero ruromero marked this pull request as ready for review January 10, 2023 15:23
bytes, err := sdkk8sutil.GetOperatorNamespace()
namespace := "local"
if err != nil {
clog.V(5).Info("Using --localOnly without --namespace, but unable to determine namespace")
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not understanding this, I would have thought "default" would be the default. Who or what interprets "local"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a placeholder for parsing the k8s-roles.properties file when not running on k8s in case the sdkk8sutil.GetOperatorNamespace() fails to retrieve the ns. That would happen only if it is running locally and not connected to a k8s cluster but that case is very unlikely to happen so I can just replace it by "" to avoid misunderstandings.

@gtully
Copy link
Contributor

gtully commented Jan 16, 2023

this is great :-)
I am wondering about the two realms, it would be ideal to be able to use the service account for broker to broker comms, like a federation connection, but that will use the "activemq" realm. Afaik, the security manager uses a single realm "activemq" by default. It is configurable, but there is only one.

Isn't federation configured with a specific credentials? https://access.redhat.com/documentation/en-us/red_hat_amq/2020.q4/html/configuring_amq_broker/assembly-br-configuring-addresses-and-queues_configuring#proc-br-configuring-upstream-address-federation_configuring I don't see how to use here the jaas configuration

yes, I am thinking future here, if the federation password could be a service account token, then that client would be authenticated with another broker against the "activemq" domain, ideally that too would have the service account login module configured.
In some way, that may be considered data plane. If it is, then treating the management domain as the only control plain jaas config will work.

however if the CR has clustered = true, we would need to be able to authenticate broker to broker comms with a service account. Either the cluster/federation acceptor can use the management domain, or we need to amend/extend the activemq realm

@gtully
Copy link
Contributor

gtully commented Jan 16, 2023

looking in more detail at this, I think we need a feature flag, otherwise all existing deployments will be restarted with the extra volume mount needing a stateful set update/refresh.

and thinking more, maybe the flag is the presence of a -jaas-config extra mount. When external jaas config is specified the operator manages its own for the control plane, ie: the "management" realm

@gtully
Copy link
Contributor

gtully commented Jan 16, 2023

and the management realm config should be in a secret rather than a config map.

@gtully
Copy link
Contributor

gtully commented Jan 16, 2023

I am thinking to split this PR:

  1. the control plane realm when -jaas-config is in play
  2. service account bits

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.

Operator should authenticate using ServiceAccount token
4 participants