-
Notifications
You must be signed in to change notification settings - Fork 380
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
Cleanup cluster roles and bindings #2938
Conversation
1926607
to
39aea78
Compare
@@ -681,7 +681,7 @@ func TestOpAMPBridgeReconciler_Reconcile(t *testing.T) { | |||
exists, err = populateObjectIfExists(t, &v1.Service{}, namespacedObjectName(naming.OpAMPBridgeService(params.Name), params.Namespace)) | |||
assert.NoError(t, err) | |||
assert.True(t, exists) | |||
exists, err = populateObjectIfExists(t, &v1.ServiceAccount{}, namespacedObjectName(naming.ServiceAccount(params.Name), params.Namespace)) | |||
exists, err = populateObjectIfExists(t, &v1.ServiceAccount{}, namespacedObjectName(naming.OpAMPBridgeServiceAccount(params.Name), params.Namespace)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was accidentally passing bc the SA was there from the previous test.
|
||
// The cluster scope objects do not have owner reference. | ||
func (r *OpenTelemetryCollectorReconciler) findClusterRoleObjects(ctx context.Context, params manifests.Params) (map[types.UID]client.Object, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given we have to do this for a fair amount of resources now, I think it may be worth it to make this a generic method that works on [T client.Object]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be a list e.g. `[]metav1.List`` but then I am not able to get the UID for the map
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you may be able to []v1.Object
(from the metav1 package)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we cannot use the client.List
and rely on labels like other methods in this file. Instead we would need to change to client.Get
and pass as well the [] types.NamespacedName
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot make the method more generic and look simple
Description:
Link to tracking Issue(s):
Testing:
Documentation: