Add custom config option for defining custom connection keys

[turkenh]

Description of your changes

This PR adds a custom configuration option for connection detail keys where provider developers could provide a go function that takes Terraform state attributes and returns a map containing custom keys/values that they want to see in the connection details secret as. With this interface, provider developers could build a connection key in any format they want, using available terraform attributes, e.g. SQL connection string as a single key, or add a nonsensitive field as connection key, like endpoint.

In this PR, we are also adding attribute_ prefix for keys of sensitive terraform fields. We needed this prefix to ensure that they are not overridden by any custom connection key configured which would break our ability to build tfstate back. Considering Terraform already lists them under Attributes, this should be intuitive.

Fixes #86

I have:

• Read and followed Crossplane's contribution process.
• Run make reviewable to ensure this PR is ready for review.
• Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

With provider-tf-aws using https://github.com/crossplane-contrib/provider-tf-aws/pull/119/files

1. Create a iam AccessKey
2. Check its connection secret

Observed a secret like where aws_access_key_id and aws_secret_access_key are custom keys:

apiVersion: v1
data:
  attribute_id: QUtJQVk0QUZUVFNFNDI2TlhKS0I=
  attribute_secret: ABCxyzRedacted==
  attribute_ses_smtp_password_v4: QQ00REDACTED==
  aws_access_key_id: QUtJQVk0QUZUVFNFNDI2TlhKS0I=
  aws_secret_access_key: ABCxyzRedacted==
kind: Secret

[turkenh]

@muvaf this should be ready to go!

[muvaf]

Thanks @turkenh ! I love how easy we're able to hook up the custom config to the secret flow we have in place here.

[muvaf]

Do we still need this field? I think supplying the function would be more preferred in almost all cases since they get to choose the keys as well.

[turkenh]

Supplying the function is not the same as this, since this configures the field as sensitive which causes it to be dropped from the schema (or replaced with a secretRef if input).

However, as we discussed offline, in the scope of the pipeline abstraction work, we will expose a configuration that allows to modify the terraform schema of fields and this could be satisfied by setting the sensitive field.

Given that currently we don't have usage of this option (in tf-aws/tf-azure) and we will have a solution to the problem that this is solving, I will remove it.

[muvaf]

Given that now we return error if user tried to override an attribute key, do we still need this?

[turkenh]

I still see some value with this, basically marking them explicitly. This would both help to identify which field is really coming from Terraform schema by default (where users can go and check the documentation to see what is expected there) and also would free keys for custom usage by not reserving common ones, like username, password etc. I cannot immediately see an example of why someone could want to provide a custom username key different than what terraform returns (maybe a tf bug 🤔 ), however, I would like to leave that room since it would be harder to switch back to this given this is kind of part of our API.

[turkenh]

Another advantage just coming to my mind is since it is hard to test with all possible configurations while adding a new resource, the provider developer could miss a terraform attribute and use it as a key but in runtime, someone testing with that specific configuration might start getting errors saying that key is reserved.

[muvaf]

Sounds good. IMHO, the users won't really care where it comes from when they use the secret to mount to a Pod or write the propagation in Composition but it's also the safer option.

[muvaf]

Do you think the . will cause any problems in connectionDetails propagation of Composition?

[turkenh]

Good question, I don't see an immediate reason to cause any problem, but you can never know without testing. But theoretically, it should work since k8s allows to have . inside secret keys.
I was just ok to use . because we will already have dots in the case of maps/lists.
So, I would prefer to go with it and think about how (e.g. replace all dots with sth else?) and where (in terrajet or in crossplane composition code) to fix it in case we see some issues.

[muvaf]

LGTM! Thanks @turkenh !