Skip to content
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

Merged
merged 53 commits into from
Jul 26, 2023
Merged

Update Composition Docs #475

merged 53 commits into from
Jul 26, 2023

Conversation

plumbis
Copy link
Collaborator

@plumbis plumbis commented Jun 22, 2023

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:

Page Reviewer1 Reviewer2
Compositions @negz
XRD @negz
XR @negz
Claims @negz
Patch and Transform @jbw976
EnvironmentConfigs @jbw976
connection details @jbw976

Signed-off-by: Pete Lumbis [email protected]

@netlify
Copy link

netlify bot commented Jun 22, 2023

Deploy Preview for crossplane ready!

Name Link
🔨 Latest commit ce9fedf
🔍 Latest deploy log https://app.netlify.com/sites/crossplane/deploys/64c15fc1f67e2b0007e0e2b1
😎 Deploy Preview https://deploy-preview-475--crossplane.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@negz
Copy link
Member

negz commented Jul 13, 2023

@plumbis Should I set this to ready for review? I think it is, right?

@plumbis
Copy link
Collaborator Author

plumbis commented Jul 13, 2023

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.

@AaronME AaronME self-assigned this Jul 13, 2023
Copy link
Member

@negz negz left a 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 Show resolved Hide resolved
```

The Claim uses the XRD's
{{<hover label="xrd1" line="10">}}claimNames{{</hover>}} to request resources.
Copy link
Member

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.

Copy link
Collaborator Author

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

Comment on lines 89 to 92
When Crossplane creates a Claim in a namespace it also creates a composite
resource.
Copy link
Member

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".

Copy link
Collaborator Author

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.

Copy link

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.

Comment on lines +138 to +141
By default, creating a Claim creates a new composite resource. Claims can also
link to existing composite resources.
Copy link
Member

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".

Copy link

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.

utils/vale/styles/Google/Headings.yml Outdated Show resolved Hide resolved
content/master/concepts/patch-and-transform.md Outdated Show resolved Hide resolved
content/master/concepts/patch-and-transform.md Outdated Show resolved Hide resolved
content/master/concepts/patch-and-transform.md Outdated Show resolved Hide resolved
content/master/concepts/patch-and-transform.md Outdated Show resolved Hide resolved
- type: FromCompositeFieldPath
fromFieldPath: spec.parameters.size
toFieldPath: spec.forProvider.settings.tier
```
Copy link
Member

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.

Copy link
Member

@jbw976 jbw976 left a 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 🙇‍♂️

content/master/concepts/compositions.md Show resolved Hide resolved
content/master/concepts/composite-resources.md Outdated Show resolved Hide resolved
Copy link
Member

@jbw976 jbw976 left a 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!

content/master/concepts/patch-and-transform.md Outdated Show resolved Hide resolved
content/master/concepts/patch-and-transform.md Outdated Show resolved Hide resolved
content/master/concepts/patch-and-transform.md Outdated Show resolved Hide resolved

The
{{<hover label="combineFromComp" line="12">}}CombineFromComposite{{</hover>}}
patch only supports the
Copy link
Member

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.

Copy link
Collaborator Author

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.

content/master/concepts/patch-and-transform.md Outdated Show resolved Hide resolved
@plumbis plumbis marked this pull request as ready for review July 21, 2023 21:36
Copy link
Member

@jbw976 jbw976 left a 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 🤓

content/master/concepts/environment-configs.md Outdated Show resolved Hide resolved
Copy link
Member

@jbw976 jbw976 left a 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.

content/master/concepts/compositions.md Show resolved Hide resolved
content/knowledge-base/guides/connection-details.md Outdated Show resolved Hide resolved
content/knowledge-base/guides/connection-details.md Outdated Show resolved Hide resolved
content/knowledge-base/guides/connection-details.md Outdated Show resolved Hide resolved
content/knowledge-base/guides/connection-details.md Outdated Show resolved Hide resolved
content/knowledge-base/guides/connection-details.md Outdated Show resolved Hide resolved
content/knowledge-base/guides/connection-details.md Outdated Show resolved Hide resolved
content/knowledge-base/guides/connection-details.md Outdated Show resolved Hide resolved
content/knowledge-base/guides/connection-details.md Outdated Show resolved Hide resolved
@AaronME AaronME self-requested a review July 25, 2023 20:12
Copy link

@AaronME AaronME left a 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
Copy link

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.

Copy link
Collaborator Author

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.

Comment on lines +243 to +258
Compositions and CompositeResourceDefinitions require the exact names of the
secrets generated by a resource.
Copy link

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.

Copy link
Collaborator Author

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.

Comment on lines +248 to +263
Resources in a Composition that create connection details still create a
secret object containing their connection details.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

Copy link
Collaborator Author

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.

Copy link

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."

Copy link
Collaborator Author

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.

content/knowledge-base/guides/connection-details.md Outdated Show resolved Hide resolved
Comment on lines +30 to +36
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.
Copy link

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).

content/master/concepts/composite-resources.md Outdated Show resolved Hide resolved
content/master/concepts/composite-resources.md Outdated Show resolved Hide resolved
usernames, passwords or connection details like an IP address.

Crossplane refers to this information as the _connection details_ or
_connection secrets_.
Copy link
Contributor

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.

Copy link
Collaborator Author

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]>
Copy link
Member

@jbw976 jbw976 left a 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!! 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants