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

✨ apiexports: make SubjectAccessReview and LocalSubjectAccessReview claimable #3129

Merged
merged 1 commit into from
May 11, 2024

Conversation

sttts
Copy link
Member

@sttts sttts commented May 10, 2024

Summary

The list of native resources that are claimable is static and defined at https://github.com/kcp-dev/kcp/blob/main/pkg/virtual/apiexport/schemas/builtin/builtin.go#L88.

This PR adds SubjectAccessReview and LocalSubjectAccessReview to that list.

Related issue(s)

Release Notes

Allow claiming `SubjectAccessReview` and `LocalSubjectAccessReview` in apiexports.

@kcp-ci-bot kcp-ci-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has signed the DCO. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 10, 2024
@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels May 10, 2024
@sttts sttts changed the title <!-- Thanks for creating a pull request! :feature: apiexports: make SubjectAccessReview and LocalSubjectAccessReview claimable May 10, 2024
@sttts sttts changed the title :feature: apiexports: make SubjectAccessReview and LocalSubjectAccessReview claimable ✨ apiexports: make SubjectAccessReview and LocalSubjectAccessReview claimable May 10, 2024
@sttts sttts force-pushed the sttts-claimable-sar branch 2 times, most recently from 4a168e5 to b6aa26c Compare May 10, 2024 11:36
@kcp-ci-bot kcp-ci-bot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 10, 2024
@sttts sttts force-pushed the sttts-claimable-sar branch 7 times, most recently from bd22196 to a668fb8 Compare May 10, 2024 14:02
Comment on lines 171 to 173
Cluster string `json:"cluster,omitempty"`
Cluster logicalcluster.Name `json:"cluster,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this a breaking change? We should add a release note in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

true. Breaking in code.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Comment on lines 145 to 147
APIExportClusterName string `json:"apiExportClusterName,omitempty"`
APIExportClusterName logicalcluster.Name `json:"apiExportClusterName,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this also a breaking change?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@sttts
Copy link
Member Author

sttts commented May 10, 2024

/approve

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 10, 2024
@mjudeikis
Copy link
Contributor

/lgtm
/hold

@kcp-ci-bot kcp-ci-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 11, 2024
@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label May 11, 2024
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 639c33e94ca9c31d969b39dfa3e9dc2f1d9b291e

@kcp-ci-bot kcp-ci-bot removed the lgtm Indicates that a PR is ready to be merged. label May 11, 2024
@kcp-ci-bot kcp-ci-bot requested a review from mjudeikis May 11, 2024 17:54
@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sttts

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

@sttts
Copy link
Member Author

sttts commented May 11, 2024

/hold cancel

@kcp-ci-bot kcp-ci-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 11, 2024
@mjudeikis
Copy link
Contributor

/lgtm

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label May 11, 2024
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 7bce21f906065ed9dd7ca3c4c78a26eda33a3d50

@kcp-ci-bot kcp-ci-bot merged commit f238118 into kcp-dev:main May 11, 2024
16 checks passed
@palnabarun
Copy link
Member

late /lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants