infinispan / infinispan-operator

Infinispan Operator
https://infinispan.org/docs/infinispan-operator/main/operator.html
Apache License 2.0
48 stars 54 forks source link

Default developer credentials Secret should be made usable as is by client app #1827

Open tux-o-matic opened 1 year ago

tux-o-matic commented 1 year ago

By default the Operator generates two Secrets:

For applications meant to connect to the Infinispan cluster, say a Quarkus Java app with the HotRod client, the content of the Secret for the "developer" user must be copied in distinct Secret (and ideally via SealedSecret or Vault in a proper CD way).

If the Secret for the "developer" user was generated exactly like the Secret for the "operator" user, the Pod for the Java client app could just reference the Secret and use the username and password as ENV values in a container which then can be interpolated natively in the Java properties file used to configure the HotRod client.

By having two different formats between the Secrets generated, manual work is required for the developers to use those credentials. If the format was the same, meaning also having simple keys for the "developer" credentials, it would be transparent to reference the Secret and the password would never have to be copied before being able to use Infinispan

ryanemerson commented 1 year ago

The "developer" secret utilises the identities.yaml format because it's necessary for users to be able to create multiple credentials, each of which can optionally have authorization roles associated with them.

Whereas the "operator" secret only requires a single user, which should only be used by the Operator itself and Prometheus integrations, hence why we can simply expose the username and password keys.

That being said @tux-o-matic, I agree that there should be a simple way for an application to automatically connect to the Infinispan cluster. The solution is for the Infinispan cluster to expose a ServiceBinding Secret that contains all of the required connection details for a specific user https://github.com/infinispan/infinispan-operator/issues/1725. Once this has been exposed, applications can consume username fields etc as ENV variables by mounting the Secret, or in the case of Quarkus they can leverage the ServiceBinding extension to automatically configure their client https://github.com/quarkusio/quarkus/issues/11821.

Proposal

Add an additional field to identities.yaml so that users can select which credential should be exposed via the ServiceBinding secret:

identities.yaml:

serviceBinding: my-user-1
credentials:
  - username: my-user-1
    password: changeme
    roles:
      - admin
  - username: my-user-2
    password: changeme
    roles:
      - monitor
tux-o-matic commented 1 year ago

Should the identities.yaml file even be involved? Shouldn't the solution be implemented via the Infinispan CRD? With a field to enable the creation of a ServiceBinding CR and in another field the user to expose, by default "developer".

ryanemerson commented 1 year ago

@tux-o-matic That's something I considered, but I wasn't sure whether it was a good idea to expose the credential username via the CR. WDYT @Crumby @rigazilla?

We could have:

spec:
  serviceBinding:
    enabled: true | false
    credential: <username>

If credential is omitted, then we default to "developer" like you suggest.

tux-o-matic commented 1 year ago

Or to integrate with the existing spec.security definition in the Infinispan CRD, adding a exposeServiceBinding boolean:

              security:
                description: InfinispanSecurity info for the user application connection
                properties:
                  authorization:
                    properties:
                      enabled:
                        type: boolean
                      exposeServiceBinding:
                         type: boolean
                      roles:
[...]

Because as far as I understand it, the CRD can be used to define custom roles as part og the spec.security.authorization section but the name "developer" of the default user is hardcoded in the Controller. You can enable or disable authorisation but if enabled (default), the end user account created is hardcoded to "developer.

ryanemerson commented 1 year ago

I don't think it belongs under .security, because a ServiceBinding is still valid even when no authorization or authentication is enabled. For example, a client might want to be automatically configured just with the host and port of the service. In such a case, the username and password fields would be omitted from the ServiceBinding Secret as authentication is disabled.

Because as far as I understand it, the CRD can be used to define custom roles as part og the spec.security.authorization section but the name "developer" of the default user is hardcoded in the Controller. You can enable or disable authorisation but if enabled (default), the end user account created is hardcoded to "developer.

The "developer" user is only used when a user doesn't provide their own credential secret via spec.security.endpointSecretName. If a user provides their own identities.yaml, then they can configure whatever usernames they want and assign the appropriate roles (defined in spec.security.authorization.roles.

Maybe the default for spec.serviceBinding.credential should simply be the first entry in identities.yaml if not explicitly configured in the spec, that way we cover both cases.

rigazilla commented 1 year ago

@tux-o-matic That's something I considered, but I wasn't sure whether it was a good idea to expose the credential username via the CR. WDYT @Crumby @rigazilla?

I'm in favour of use Infinispan CR for this but I agree that for safety reason username should be in a secret, so maybe:

spec:
  serviceBinding:
    enabled: true | false
    secretRef: <secret name>  // if missing go for the first username in identities.yaml
ryanemerson commented 1 year ago

I would rather the user didn't have to create an additional Secret just for this purpose.

Maybe we can have a hybrid approach, whereby the ServiceBinding is enabled via the CR but the user to expose is configured in the existing credential secret?

So it would be:

CR:

spec:
  serviceBinding:
    enabled: true | false # Defaults to true

Secret specified via spec.security.endpointSecretName:

serviceBindingUser: my-user-1
identities.yaml: |
  credentials:
    - username: my-user-1
      password: changeme

If the user doesn't provide a Secret via spec.security.endpointSecretName, then we use the "developer" user as we know that's the only user configured as we generated the credential. This way, a user who just wants to consume the cluster without any additional configuration doesn't have to do anything and the ServiceBinding will be created by default.

rigazilla commented 1 year ago

I would rather the user didn't have to create an additional Secret just for this purpose.

Maybe we can have a hybrid approach, whereby the ServiceBinding is enabled via the CR but the user to expose is configured in the existing credential secret?

So it would be:

CR:

spec:
  serviceBinding:
    enabled: true | false # Defaults to true

Secret specified via spec.security.endpointSecretName:

serviceBindingUser: my-user-1
identities.yaml: |
  credentials:
    - username: my-user-1
      password: changeme

If the user doesn't provide a Secret via spec.security.endpointSecretName, then we use the "developer" user as we know that's the only user configured as we generated the credential. This way, a user who just wants to consume the cluster without any additional configuration doesn't have to do anything and the ServiceBinding will be created by default.

yep this solution is better, I was worried about adding things inside the identities.yaml...

(Just for clarity: I was considering the secret creations a task of the admin not something in charge of the consumer)

Do we plan to provide something more multi-tenant? I mean something like serviceBindingRW, serviceBindingRO ? In case, can this solution be easily extended ?

ryanemerson commented 1 year ago

Do we plan to provide something more multi-tenant? I mean something like serviceBindingRW, serviceBindingRO ? In case, can this solution be easily extended ?

The ServiceBinding Operator doesn't allow such multi-tenancy when binding to a CR. The secret providing the connection details is established by inspecting the status.binding.name field. This secret is then bound to the consuming application.

If the user requires a more complex setup like you describe, they could create multiple Secrets and ServiceBindings explicitly, but I don't think we can automatically provision such resources as it's very much use-case dependent.

rigazilla commented 1 year ago

If the user requires a more complex setup like you describe, they could create multiple Secrets and ServiceBindings explicitly, but I don't think we can automatically provision such resources as it's very much use-case dependent.

Oh ok. So imo, your proposal sound good enough for this issue

tux-o-matic commented 1 year ago

There shouldn't be passwords in the Infinispan CR, that would go against the practice of encrypting passwords and tokens being fetched from Git when talking Continuous Deployment (and I opened this issue to improve CD experience for developers). And I'm not even sure SealedSecret or Hashicorp Vault make it possible to use encrypted CustomResource.