-
Notifications
You must be signed in to change notification settings - Fork 110
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
Update Composition Docs #475
Conversation
✅ Deploy Preview for crossplane ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@plumbis Should I set this to ready for review? I think it is, right? |
The published pages mentioned in the first comment are all ready for review. There are still at least two pages to write/edit before this would be ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @plumbis! I like where this is headed.
None of my comments are major or blocking - happy for someone else to approve this once you're ready.
content/master/concepts/claims.md
Outdated
``` | ||
|
||
The Claim uses the XRD's | ||
{{<hover label="xrd1" line="10">}}claimNames{{</hover>}} to request resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by "request resources" here?
I think of claimNames
is defining the type (i.e. type name) of the claim.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this to "uses the XRD's kind
API endpoint to request resources"
I'm purposefully vague with "request resources" to avoid unnecessary details about the underlying XR
content/master/concepts/claims.md
Outdated
When Crossplane creates a Claim in a namespace it also creates a composite | ||
resource. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: This reads to me as if Crossplane is going to, itself, create my-claimed-database
.
I would think of what's happening here as something more like:
"When you create my-claimed-database
in a namespace, Crossplane creates a matching XR".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with "you" here is that it's not necessarily the reader. Flipping between terms (you, user, developer, etc) makes the reader have to "keep state" against who is doing what.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to second Nic's point. Crossplane has no ability to create claims. Claims are always user-applied.
By default, creating a Claim creates a new composite resource. Claims can also | ||
link to existing composite resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For context: This is where the name "claim" comes from. "I'm claiming this XR".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have any real-world examples of users doing this? I'm just curious.
- type: FromCompositeFieldPath | ||
fromFieldPath: spec.parameters.size | ||
toFieldPath: spec.forProvider.settings.tier | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: I reviewed every file but this one, but only got this far down this one before I had to run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just took a pass specifically through the connection secret details in the context of #485, that's all I looked at right now 🙇♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a full pass through patch-and-transform.md
today and left some comments, looking very good @plumbis!
|
||
The | ||
{{<hover label="combineFromComp" line="12">}}CombineFromComposite{{</hover>}} | ||
patch only supports the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for a few of these patch types, we say that it only supports X option, then further down after another paragraph we say that it only supports Y strategy. Should these two sentences be combined into the same paragraph, so the context of "limitations" for this patch type is all grouped together?
e.g., The CombineToComposite patch only supports the combine
option and the only supported strategy
is strategy: string
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give me another example? For this one (and likely the others) I keep them on unique lines to try and make it easier to find the one line/argument/whatever when you're skimming or to be able to understand each line (almost like an API doc) if you're building it for the first time. Open to changing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finished reading/reviewing the Environment Configurations page! Not too many comments here, mostly it was all helpful/useful 🤓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read through the new connection details KB page and the storing connection details section of the compositions page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @plumbis !
This guide discusses creating Kubernetes secrets. | ||
Crossplane also supports using external secret stores like [HashiCorp Vault](https://www.vaultproject.io/). | ||
|
||
Read the [external secrets store guide]({{<ref "/knowledge-base/integrations/vault-as-secret-store">}}) for more information on using Crossplane |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be helpful to call out that using ESS or native secrets is a decision which must be made at composition design time. The configuration fields are mutually exclusive, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't tested it but according to code comments you can do both.
Compositions and CompositeResourceDefinitions require the exact names of the | ||
secrets generated by a resource. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what this is saying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm struggling to express "hey, this is how you find the names of the keys a resource produces and you'll need them when defining the connectiondetails
in a Composition"
I'm wide open to suggestions.
Resources in a Composition that create connection details still create a | ||
secret object containing their connection details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resources in a Composition that create connection details still create a | |
secret object containing their connection details. | |
Managed Resources create secrets containing their connection details. This is true whether the Managed Resource is declared on its own or as part of a Composition, and whether that Composition is called as an XR or Claim. |
Indicating that this is Managed Resource functionality might help users grok the crossplane/kubernetes model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you trying to say "an MR creates a secret. This always happens, even when it's an XR?"
Crossplane also generates | ||
another secret object for each composite resource, | ||
containing the secrets from all the defined resources. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Composition authors will need to be aware of both of these secrets when crafting their compositions."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's necessarily true. it's possible that the composed resource secret isn't used/needed and only the composite secret is used.
Creating composite resources requires a | ||
[Composition]({{<ref "./compositions">}}) and a | ||
[CompositeResourceDefinition]({{<ref "./composite-resource-definitions">}}) | ||
(`XRD`). | ||
The Composition defines the set of resources to create. | ||
The XRD defines the custom API users call to request the set of resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've leaned into the following convention, because XR can get confusing:
I use the term "Composite Resource" to refer to the API defined by an XRD and one or more Compositions.
I use the term "CompositeResource" and the abbreviation "XR" to refer to a Resource which calls the API of a Composite Resource -- or a specific instantiation of a Composite Resource.
I welcome any suggestions on how to more clearly differentiate between the API which can be called (XRD + Composition = Composite Resource) and a specific call to that API (Claim --> CompositeResource, or XR).
usernames, passwords or connection details like an IP address. | ||
|
||
Crossplane refers to this information as the _connection details_ or | ||
_connection secrets_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two paragraphs should better be right at the top to set the stage what this topic is actually about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason it's not is because the current top is intended to be a quick reference for anyone dealing with secrets and wanting a checklist to see what they did right/wrong.
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
Signed-off-by: Pete Lumbis <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all the collaboration and syncing together to review and discuss all these changes. They will be a great improvement for the v1.13 docs!! 💪
This PR rewrites the docs related to Composition.
The new /concepts/compositions page streamlines the details and focuses on the fields available in a Composition and what they do.
New XRD page breaks out info specific to writing XRDs and XRD status. Resolves #456, #448, #288,
New XR page details XR options. Resolves #455, (partially) #370, #432.
New Claims page details creating a Claim and claiming an existing XR. Resolves #281.
New Patch and Transform page covering patching options and configurations in detail. Resolves #380
New documentation on EnvironmentConfigs. Resolves #305
Additionally composite functions has been moved out of the KB and into
/concepts
New KB on connection details.
Reviewed:
Signed-off-by: Pete Lumbis [email protected]