Implement ServiceAccountKey managed resource support

[dgorender]

What problem are you facing?

Crossplane create a service account alright, but there is no option to generate a credentials key. The yaml has a writeConnectionSecretToRef attribute, but it does not write anything into the secret.

How could Crossplane help solve your problem?

By creating an option for storing service accounts credential in a secret, be it via service account atributtes or another managed resource.

[muvaf]

I think we need a separate managed resource called ServiceAccountKey that would write public and private key to a secret. An example from docs is here. I'll change the title accordingly.

[ulucinar]

Hi,
I will try to take care of this issue. Thanks for reporting.

[ulucinar]

I did some experiments for provisioning Google Cloud IAM service account keys. It seems to be the case that GCP does not persist the provisioned private key [1], thus provides no APIs to get the private key after it's provisioned. The provisioning API, however, does allow uploading public keys for service accounts [2]. Thus my proposal would be to:

• always generate the key pair in the controller, and
• persist it in a K8s secret, and then
• upload the public key for the SA via the projects.serviceAccounts.keys.upload [3] method. What do you think? cc. @muvaf, @turkenh, @hasheddan
  Do we already have a utility library to generate key pairs?
  [1] https://cloud.google.com/iam/docs/service-accounts#user-managed_keys
  [2] https://cloud.google.com/iam/docs/creating-managing-service-account-keys#uploading
  [3] https://cloud.google.com/iam/docs/reference/rest/v1/projects.serviceAccounts.keys/upload

[muvaf]

It seems like Create API allows us to toggle the KeyAlgorithm and PrivateKeyType fields. In order to achieve high fidelity, we need to expose these fields in CRD and configure the generated key payload with that information. But I am not sure if we want to get to that deep into the weeds because the more custom operations like this one we do, the harder it gets for code generation (or migration to code-generated resources) in the future.

I'd like to step back and think about why we need this. Seems like we're worried about the case where we create the key successfully but fail to save the returned private key to the connection Secret here, hence lose it. IMO, we need to handle this case better in the crossplane-runtime because it's a generic case and in the future we might not have the chance to use an upload call like this.

I'd say let's keep the implementation simple now, rely on crossplane-runtime's handling of the connection secrets and open an issue there to solve this for all resources (even if it can never be a 100% guaranteed solution).

[ulucinar]

It seems like Create API allows us to toggle the KeyAlgorithm and PrivateKeyType fields. In order to achieve high fidelity, we need to expose these fields in CRD and configure the generated key payload with that information. But I am not sure if we want to get to that deep into the weeds because the more custom operations like this one we do, the harder it gets for code generation (or migration to code-generated resources) in the future.

Thanks for mentioning the high-fidelity requirements. As we discussed, we can keep the Create API intact, for now, as the parameters, their default values and semantics are well-defined and feasible to implement on the client (crossplane) side. But this is also added complexity for us. We would need to add support for any new GCP-supported key algorithms, or output formats in the future.

I'd like to step back and think about why we need this. Seems like we're worried about the case where we create the key successfully but fail to save the returned private key to the connection Secret here, hence lose it. IMO, we need to handle this case better in the crossplane-runtime because it's a generic case and in the future we might not have the chance to use an upload call like this.

Yes, exactly. Potentially we will leak keys for a service account. No security concerns though because we can assume the associated private keys will be gone forever in case we lose them. Just from a resource consumption point of view. For instance, if I remember correctly, there is 10 key/SA limit on GCP. And because we cannot put provisioning of the SA key and the K8s secret inside the same transaction boundary, we cannot guarantee that we won't leak as we have discussed. But this is a compromise we can make.

I'd say let's keep the implementation simple now, rely on crossplane-runtime's handling of the connection secrets and open an issue there to solve this for all resources (even if it can never be a 100% guaranteed solution).

Agreed. I believe we should have encountered similar situations in the past and I wanted to learn which direction we have chosen so far in those similar cases. Thanks for sharing insights @muvaf.

[hasheddan]

@muvaf I had a good chat with @ulucinar this morning and I think a simple solution for trying to ensure the connection Secret gets populated when connection details are returned from the Create call would be to duplicate the retry logic you added here in the error handling for connection Secret below it. What do you think?

[muvaf]

Sounds good to me but I think it should happen in the implementation of the publisher, rather than the managed reconciler. Because supposedly, the underlying publishing mechanism might not be Kubernetes secret, hence we can't know the error type.