Skip to content

Prevent cluster groups from being deleted when referenced by a projects' configuration #15119

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

Merged
merged 10 commits into from
Mar 17, 2025

Conversation

markylaing
Copy link
Contributor

Adds an API extension clustering_groups_used_by and adds a UsedBy field to api.ClusterGroup. On GET requests, the UsedBy URLs are projects that contain the cluster group in restricted.cluster.groups. DELETE requests will fail if the cluster group is used by a project.

Closes #15118

@markylaing markylaing added the Bug Confirmed to be a bug label Mar 6, 2025
@markylaing markylaing self-assigned this Mar 6, 2025
@github-actions github-actions bot added Documentation Documentation needs updating API Changes to the REST API labels Mar 6, 2025
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Looks good, except the SQL injection flaw ;)

@markylaing
Copy link
Contributor Author

Looks good, except the SQL injection flaw ;)

Whoopsie 😆 Thank you CodeQL 🙏 Not sure how I convinced myself that this was ok...

@markylaing markylaing force-pushed the cluster-group-used-by branch 2 times, most recently from bac19b8 to 2e939e5 Compare March 14, 2025 12:11
@markylaing markylaing force-pushed the cluster-group-used-by branch from 2e939e5 to c78bd05 Compare March 14, 2025 16:14
@markylaing markylaing requested a review from tomponline March 14, 2025 16:14
@tomponline
Copy link
Member

Static analysis failure

Signed-off-by: Mark Laing <[email protected]>

# Conflicts:
#	doc/api-extensions.md
#	shared/version/api.go
This duplicates `(*cluster.ClusterGroup).ToAPI`.

Signed-off-by: Mark Laing <[email protected]>
This function queries for projects that have the group name
present in the value for the config key `restricted.cluster.groups`.
Then it parses the config value properly to check for an exact match.

Signed-off-by: Mark Laing <[email protected]>
When calling `make` we should have the form:

    make([]<type>, 0,<expected_length>)

and then append to it. Otherwise we risk having uninitialised elements
(e.g. by accidentally appending instead).

Signed-off-by: Mark Laing <[email protected]>
This includes a small refactor of `GET /1.0/cluster/groups`
where the handler was unnecessarily iterating over the list of
groups twice.

Signed-off-by: Mark Laing <[email protected]>
@markylaing markylaing force-pushed the cluster-group-used-by branch from c78bd05 to 9c38fc5 Compare March 17, 2025 05:47
@markylaing
Copy link
Contributor Author

Static analysis failure

Fixed.

Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Thanks!

@tomponline tomponline merged commit 835fe31 into canonical:main Mar 17, 2025
2 checks passed
tomponline added a commit that referenced this pull request Apr 15, 2025
Follow up to #15119

Cluster group used-by URLs we're not being filtered by what the caller
is able to view. This allowed restricted users to see the URLs of
projects that they do not have access to.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes to the REST API Bug Confirmed to be a bug Documentation Documentation needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

project config 'restricted.cluster.groups' not checked when deleting a cluster group
2 participants