make providerConfigName optional and default

csantanapr commented 1 year ago

What does this PR do?

Motivation

When an user that is creating a claim which is a namespaced resource, she might not have cluster scope access to know the name of the provider config. The provider config name should not be a concern required to be exposed to the claim.
If a providerConfig name is not provider crossplane will default to the value default this is an option to the platform team to create the resource with this name or not.
The user can always provide a providerConfig name and this is optional

More

[x] Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
[ ] Yes, I have added a new example under examples to support my PR
[ ] Yes, I have updated the docs for this feature
[ ] Yes, I have linked to an issue or feature request (applicable to PRs that solves a bug or a feature request)

Note:

Not all the PRs require examples and docs
We prefer small, well tested pull requests. Please ensure your pull requests are self-contained, and commits are squashed

For Moderators

[ ] E2E Test successfully complete before merge?

Additional Notes

csantanapr commented 1 year ago

Leaving as Draft PR since the terraform module updates are not merged to allow to name the provider config default https://github.com/aws-ia/terraform-aws-eks-blueprints/pull/1220

csantanapr commented 1 year ago

@nimakaviani @nabuskey PTAL when you have a chance, this is ready now

nabuskey commented 1 year ago

My intent for explicitly specifying provider config name was to make users new to the AWS providers aware of the connection between a ProviderConfig and a AWS IAM role / K8s role. ProviderConfig is one of three CRDs that are common to almost all providers (comes with upjet and the provider template). I think it's important for users to understand why this exists and should be careful how it is configured.

I see that exposing provider config name is not friendly to end users, but I am not sure making everything default is a good idea. I agree that making it default is useful for people to have the "just works" experience but it comes with the cost of possibilities that users not understanding the concept of ProviderConfigs. Perhaps we should have a documentation or comments about how to abstract that away or use naming conventions to populate the field. I am open to other suggestions / counter points.

csantanapr commented 1 year ago

I think we should have it out of the box use default and then have an example for platform teams that explain what's the role of providerConfig and some pattern on how to use them for like different credentials to use, how to use a naming convention of using the namespace of the claim for multi-tenancy.

Currently this repository has an inconsistency that the kubernetes provider config is name default and the aws provider config is name aws-provider-config

We have an example using a composition using both providers , but it doesn't handle the kubernetes provider config name

nabuskey commented 1 year ago

Currently this repository has an inconsistency that the kubernetes provider config is name default and the aws provider config is name aws-provider-config

Yeah I agree. That was my mistake. The reason it uses default is everything related to Kubernetes is assumed to be deployed to the cluster that was created as part of the bootstrap process. I should have expanded the interface.

I think we should have it out of the box use default and then have an example for platform teams that explain what's the role of providerConfig and some pattern on how to use them for like different credentials to use, how to use a naming convention of using the namespace of the claim for multi-tenancy.

I am fine with this. Can you update this PR and add the doc explaining this?

nabuskey commented 1 year ago

Closing this for now. Feel free to reopen

awslabs / crossplane-on-eks