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

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

Open
tux-o-matic opened this issue May 25, 2023 · 11 comments

Comments

@tux-o-matic
Copy link

By default the Operator generates two Secrets:

  • One for a "developer" user generated as a YAML file mounted in the Infinispan Pods.
  • One for an "operator" user generated as a YAML file mounted in the Infinispan Pods and separate username and password keys.

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
Copy link
Contributor

ryanemerson commented May 30, 2023

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 #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 quarkusio/quarkus#11821.

Proposal

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

  • If no credentials are selected, then the username and password fields are omitted from the binding.
  • A generated user credential will always be set as the ServiceBinding value.

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
Copy link
Author

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
Copy link
Contributor

@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
Copy link
Author

tux-o-matic commented May 30, 2023

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
Copy link
Contributor

ryanemerson commented May 30, 2023

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
Copy link
Collaborator

@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
Copy link
Contributor

ryanemerson commented May 30, 2023

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
Copy link
Collaborator

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
Copy link
Contributor

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
Copy link
Collaborator

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

@rigazilla rigazilla reopened this May 31, 2023
@tux-o-matic
Copy link
Author

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.

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

No branches or pull requests

3 participants