Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Figure out how to handle sensitive information for managed resources #35

Closed
turkenh opened this issue Aug 26, 2021 · 3 comments · Fixed by #77
Closed

Figure out how to handle sensitive information for managed resources #35

turkenh opened this issue Aug 26, 2021 · 3 comments · Fixed by #77
Assignees
Labels
alpha enhancement New feature or request

Comments

@turkenh
Copy link
Member

turkenh commented Aug 26, 2021

What problem are you facing?

Crossplane provides a way to connect to cloud resources (if applicable) by writing the required details to connect into a Kubernetes secret.
These details should include all fields that would need to be required to connect including sensitive ones like username and password and not necessarily sensitive ones like port information.

How could Terrajet help solve your problem?

We would need to define/implement a way to generate these connection details with terrajet.
I am assuming this information could be composed using the attributes and sensitive_attributes fields in terraform state.

@muvaf
Copy link
Member

muvaf commented Sep 24, 2021

I think there are two issues regarding sensitive information:

  1. Output returned from the providers that is stored in tfstate.
  2. Input from user as a configuration parameter.

The tricky part is that there are cases where without implementing (2), we can't really remove the field. For example, RDS instance has a field to take password. We can copy the content of password field from tfstate into our connection detail but we can't remove the field from schema because it's used as input to the creation call.

We can possibly either expand the scope of this issue to also generate and process *v1.SecretKeyReference fields hence remove the sensitive fields completely, or keep the scope as is and don't remove the fields, only make sure that their content in tfstate makes it to the connection detail.

@turkenh
Copy link
Member Author

turkenh commented Sep 24, 2021

Good point. Originally, I was thinking the scope of this issue is more around the first one.
But the second could be solved by following a similar solution as we used to mark and generate reference fields, e.g. developer marks the field as sensitive and terrajet adds a secretref field if it is input together with providing function to read it. I don't think we would need a special code generation like reference resolvers though since this case would be simpler.

if field is marked as sensitive & input (i.e. from spec) => generate a SecretKeyRef field and read from the referenced secret in GetParameters.
if field is marked as sensitive & output (i.e. to status) => store in connection details secret in SetObservation (may be also store input secret to connection details).

Considering the possible flow above, I think it makes sense to handle both cases with this ticket.

@turkenh turkenh changed the title Figure out how to fill connection details for managed resources Figure out how to handle sensitive information for managed resources Sep 24, 2021
@muvaf muvaf added alpha and removed post-alpha labels Sep 24, 2021
@muvaf
Copy link
Member

muvaf commented Sep 26, 2021

generate a SecretKeyRef field and read from the referenced secret in GetParameters.
store in connection details secret in SetObservation (may be also store input secret to connection details).

We'd have to add kube client.Client argument to GetParameters function and return a map[string][]byte from SetObservation, right?

I think it starts to feel like we're overloading the functions that seem very basic on the outside, i.e. classic getters and setters, with some logic that are normally handled elsewhere in manually written controllers.

I wonder if we can extend Terraformed interface call those functions in the controller itself. For example:

func (r *RDSInstance) GetConnectionDetails(tfstate map[string]interface{}) (map[string][]byte, error)
func (r *RDSInstance) GetSensitiveParameters(kube client.Client) (map[string]interface{}, error)

What do you think? I don't really love the fact that we'd have to import controller-runtime in apis packages (we always try to keep what's imported there at minimum so that depending on our apis is not high-cost) but I couldn't find a nice way of having those functions in clients or controllers package while keeping the configuration input in one place, i.e. resource.Configuration object used in apis/group/*. And it feels early to have the controller.Configuration object we had talked about that can be used for non-apis logic.

@luebken luebken added this to the Terrajet-Alpha milestone Sep 27, 2021
@turkenh turkenh mentioned this issue Sep 29, 2021
13 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
alpha enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants