Skip to content

Conversation

@freeznet
Copy link
Member

(If this PR fixes a github issue, please add Fixes #<xyz>.)

Fixes #

(or if this PR is one task of a github issue, please add Master Issue: #<xyz> to link to the master issue.)

Master Issue: #

Motivation

Explain here the context, and why you're making that change. What is the problem you're trying to solve.

Modifications

Describe the modifications you've done.

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Documentation

Check the box below.

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

freeznet added 4 commits July 10, 2025 12:39
- Added RoleBinding controller to manage RoleBinding resources in Kubernetes.
- Updated RoleBindingSpec to change fields from single string to array of strings for better flexibility.
- Enhanced CRD definitions to reflect the new array types for SRN fields.
- Introduced deep copy methods for RoleBindingList and ClusterRoleList to support list operations.
- Added documentation for RoleBinding resource, including examples and specifications.
@freeznet freeznet self-assigned this Jul 10, 2025
@freeznet freeznet requested review from a team as code owners July 10, 2025 13:11
@github-actions
Copy link
Contributor

@freeznet:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@github-actions github-actions bot added the doc-info-missing This pr needs to mark a document option in description label Jul 10, 2025
@jiangpengcheng
Copy link
Member

the operator is pulsar-resource-operator which seems used to create pulsar resources, but the role binding is cloud resources, will this cause any mis-understanding?

@freeznet
Copy link
Member Author

the operator is pulsar-resource-operator which seems used to create pulsar resources, but the role binding is cloud resources, will this cause any mis-understanding?

We are planning to make this operator as StreamNative resources operator in future, and currently we already have multiple resources supported:

StreamNativeCloudConnection
ComputeWorkspace
ComputeFlinkDeployment
StreamNative Cloud Secret
StreamNative Cloud APIKey
StreamNative Cloud ServiceAccount
StreamNative Cloud ServiceAccountBinding

@freeznet freeznet requested a review from Copilot July 10, 2025 14:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds initial support for StreamNative Cloud RBAC resources, including RoleBinding and ClusterRole.

  • Implements a RoleBindingClient with CRUD and watch capabilities for cloud RoleBindings.
  • Generates informers, listers, and clientsets for RoleBinding and ClusterRole.
  • Extends the controller, CRDs, and documentation to handle the new RBAC resources.

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/streamnativecloud/rolebinding_client.go Client methods for RoleBinding operations
controllers/rolebinding_controller.go Reconciler logic for RoleBinding lifecycle
pkg/streamnativecloud/apis/cloud/v1alpha1/rolebinding_types.go CRD type definitions for RoleBinding
config/crd/bases/resource.streamnative.io_rolebindings.yaml CRD schema for RoleBinding
docs/rolebinding.md User guide and examples for RoleBinding
Comments suppressed due to low confidence (4)

controllers/rolebinding_controller.go:23

  • [nitpick] Consider using a more descriptive import alias than controllers2 (e.g., cloudclient) to clarify its purpose.
	controllers2 "github.com/streamnative/pulsar-resources-operator/pkg/streamnativecloud"

pkg/streamnativecloud/rolebinding_client.go:79

  • Add unit tests for convertToCloudRoleBinding and makeResourceNames to verify correct transformation of local RoleBinding specs into cloud API objects.
func convertToCloudRoleBinding(roleBinding *resourcev1alpha1.RoleBinding, organization string) *cloudapi.RoleBinding {

docs/rolebinding.md:57

  • Update the type of status.failedClusters to match the CRD ([]FailedCluster objects with name, namespace, and reason), not plain strings.
| `status.failedClusters` | []string | A list of clusters where applying the role binding failed. |

docs/rolebinding.md:58

  • Correct the type of status.syncedClusters in the docs to map[string]int64 to align with the CRD's observedGeneration values.
| `status.syncedClusters` | map[string]string | A map of clusters where the role binding has been successfully synced. The key is the cluster name and the value is the sync status. |

}

// makeResourceNames creates ResourceName array from SRN fields
func makeResourceNames(spec resourcev1alpha1.RoleBindingSpec, organization string) []cloudapi.ResourceName {
Copy link

Copilot AI Jul 10, 2025

Choose a reason for hiding this comment

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

[nitpick] The ResourceName type includes an APIKey field but it's never set here. Either populate it or remove the field to avoid confusion.

Copilot uses AI. Check for mistakes.
@freeznet freeznet merged commit 07bf3fb into main Jul 11, 2025
5 checks passed
@freeznet freeznet deleted the freeznet/support-sncloud-rbac-resources branch July 11, 2025 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-info-missing This pr needs to mark a document option in description

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants