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.

Sign up for GitHub

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

Jump to bottom

📖 Add APIBindings example #3207

Merged
merged 5 commits into from
Mar 26, 2025
Merged

📖 Add APIBindings example #3207

merged 5 commits into from
Mar 26, 2025
+59 −10

Conversation

SimonTheLeg
Copy link
Contributor

Summary

Adds an APIBinding example to fit with the example previously on the page and adds some more information on APIBindings/turns some of the bullets into full text.

The asciiflow link did not resolve for me and is also not rendered in the documentation at the moment, so I removed it.

Related issue(s)

Fixes #

Release Notes

NONE

@kcp-ci-bot kcp-ci-bot added release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. dco-signoff: no Indicates the PR's author has not signed the DCO. labels Dec 6, 2024
@kcp-ci-bot
Copy link
Contributor

Hi @SimonTheLeg. Thanks for your PR.

I'm waiting for a kcp-dev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kcp-ci-bot kcp-ci-bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 6, 2024
@kcp-ci-bot kcp-ci-bot added dco-signoff: yes Indicates the PR's author has signed the DCO. and removed dco-signoff: no Indicates the PR's author has not signed the DCO. labels Dec 6, 2024
@sttts
Copy link
Member

sttts commented Dec 6, 2024

/ok-to-test

@kcp-ci-bot kcp-ci-bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 6, 2024
@kcp-ci-bot kcp-ci-bot added dco-signoff: no Indicates the PR's author has not signed the DCO. and removed dco-signoff: yes Indicates the PR's author has signed the DCO. labels Dec 10, 2024
@kcp-ci-bot kcp-ci-bot added dco-signoff: yes Indicates the PR's author has signed the DCO. and removed dco-signoff: no Indicates the PR's author has not signed the DCO. labels Dec 10, 2024
@SimonTheLeg
Copy link
Contributor Author

addressed all comments. Ready for review again

embik
embik requested changes Dec 17, 2024
View reviewed changes
Copy link
Member

@embik embik left a comment

Choose a reason for hiding this comment

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

Some small changes I would suggest.

janwillies
janwillies reviewed Dec 17, 2024
View reviewed changes
docs/content/concepts/apis/exporting-apis.md Outdated
path: "root:api-provider" # path of your api-provider workspace
```

It should be noted that `APIBindings` do not create `CRDs` or `APIResourceSchemas`. Instead APIs are directly bound.
Copy link

@janwillies janwillies Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Instead APIs are directly bound.

What does this mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe something "apis are reflected". It there, but not in the same way in Kubernetes. So some clearing the waters is needed

Copy link
Contributor Author

@SimonTheLeg SimonTheLeg Mar 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I have re-worked this section again. I would really like to keep the word "bound" in as it comes directly from how Kubernetes behind the scenes calls it "binding apis".

I have made it more clear that this is something kcp does for you behind the scenes and then in the next sentence I outlined what the practical implications of it are.
I think it is important in this section that we don't go into too many details and only outline what this means for an end-user.

jmcshane
jmcshane reviewed Dec 19, 2024
View reviewed changes
docs/content/concepts/apis/exporting-apis.md Outdated

`APIBindings` are used to import API resources. They contain a reference to an `APIExport` using the namespace and workspace path of an `APIExport` and will bind all APIs defined in the `APIExport`. The reference path needs to be provided to you by the provider of the API or an external catalog solution.

Furthermore, `APIBindings` provide the `APIExport` owner access to additional resources defined in an `APIExport`'s `PermissionClaims` list. `PermissionClaims` must be accepted by the user explicitly, before this access is granted. The resources can be builtin Kubernetes resources or resources from other `APIExports`.
Copy link

Choose a reason for hiding this comment

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

How are these permissions granted? Need an example in line with the APIExport sample above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

very good point. I have added an example which matches the previous APIExport

@kcp-ci-bot kcp-ci-bot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 26, 2025
SimonTheLeg and others added 3 commits March 26, 2025 17:00
@SimonTheLeg
Add APIBindings example

Verified

This commit was signed with the committer’s verified signature.
SimonTheLeg Simon Bein
GPG key ID: 8A8A68258B4E118E
Verified
Learn about vigilant mode
374357a 
On-behalf-of: SAP [email protected]
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg @sttts
Apply suggestions from code review

Verified

This commit was signed with the committer’s verified signature.
SimonTheLeg Simon Bein
GPG key ID: 8A8A68258B4E118E
Verified
Learn about vigilant mode
0886ff2 
Co-authored-by: Dr. Stefan Schimanski <[email protected]>
On-behalf-of: SAP [email protected]
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg
add info on PermissionClaim on ApiExport update

Verified

This commit was signed with the committer’s verified signature.
SimonTheLeg Simon Bein
GPG key ID: 8A8A68258B4E118E
Verified
Learn about vigilant mode
2e13794 
On-behalf-of: SAP [email protected]
Signed-off-by: Simon Bein <[email protected]>
@kcp-ci-bot kcp-ci-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 26, 2025
@SimonTheLeg
address review comments

Verified

This commit was signed with the committer’s verified signature.
SimonTheLeg Simon Bein
GPG key ID: 8A8A68258B4E118E
Verified
Learn about vigilant mode
Loading
Loading status checks…
19c1877 
On-behalf-of: SAP <[email protected]>
Signed-off-by: Simon Bein <[email protected]>
@SimonTheLeg
Copy link
Contributor Author

@embik I think this PR is ready for review/merging now. I have addressed all the comments and I think it's a good state to merge.
More improvements can be added in additional PRs

embik
embik requested changes Mar 26, 2025
View reviewed changes
Copy link
Member

@embik embik left a comment

Choose a reason for hiding this comment

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

One last (🤞🏻 ) thing.

@SimonTheLeg @embik
align PermissionClaims formating

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
Loading
Loading status checks…
fc4cd59 
Co-authored-by: Marvin Beckers <[email protected]>
embik
embik approved these changes Mar 26, 2025
View reviewed changes
Copy link
Member

@embik embik left a comment

Choose a reason for hiding this comment

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

/approve

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 26, 2025
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 9f63413f9915447f0bc975bff54e9770f4679e02

@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: embik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 26, 2025
@SimonTheLeg
Copy link
Contributor Author

/cherry-pick release-0.27

@kcp-ci-bot
Copy link
Contributor

@SimonTheLeg: once the present PR merges, I will cherry-pick it on top of release-0.27 in a new PR and assign it to you.

In response to this:

/cherry-pick release-0.27

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kcp-ci-bot kcp-ci-bot merged commit fc9fa55 into kcp-dev:main Mar 26, 2025
9 checks passed
@kcp-ci-bot
Copy link
Contributor

@SimonTheLeg: new pull request created: #3355

In response to this:

/cherry-pick release-0.27

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kcp-ci-bot kcp-ci-bot mentioned this pull request Mar 26, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts

@janwillies janwillies

@mjudeikis mjudeikis

@jmcshane jmcshane

@embik embik

Assignees

@embik embik

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@SimonTheLeg @kcp-ci-bot @sttts @janwillies @mjudeikis @jmcshane @embik