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

refactor: cleanup tekton operands #641

Merged
merged 1 commit into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 17 additions & 27 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,33 +25,6 @@ rules:
- list
- update
- watch
- apiGroups:
- '*'
resources:
- configmaps
verbs:
- create
- delete
- list
- watch
- apiGroups:
- '*'
resources:
- persistentvolumeclaims
verbs:
- '*'
- apiGroups:
- '*'
resources:
- pods
verbs:
- create
- apiGroups:
- '*'
resources:
- secrets
verbs:
- '*'
- apiGroups:
- admissionregistration.k8s.io
resources:
Expand Down Expand Up @@ -153,6 +126,15 @@ rules:
- infrastructures
verbs:
- get
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
- delete
- list
- watch
- apiGroups:
- ""
resources:
Expand Down Expand Up @@ -180,6 +162,7 @@ rules:
resources:
- persistentvolumeclaims
verbs:
- '*'
- create
- delete
- get
Expand Down Expand Up @@ -208,9 +191,16 @@ rules:
resources:
- pods
verbs:
- create
- get
- list
- watch
- apiGroups:
- ""
resources:
- secrets
verbs:
- '*'
- apiGroups:
- ""
resources:
Expand Down
6 changes: 4 additions & 2 deletions controllers/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,14 @@ func setupManager(ctx context.Context, cancel context.CancelFunc, mgr controller
return fmt.Errorf("failed to read vm-console-proxy bundle: %w", err)
}

tektonPipelinesBundle, err := tekton_bundle.ReadPipelineBundle()
tektonPipelinesBundlePaths := tekton_bundle.GetTektonPipelineBundlePaths()
tektonPipelinesBundle, err := tekton_bundle.ReadBundle(tektonPipelinesBundlePaths)
if err != nil {
return err
}

tektonTasksBundle, err := tekton_bundle.ReadTasksBundle(runningOnOpenShift)
tektonTasksBundlePath := tekton_bundle.GetTektonTasksBundlePath(runningOnOpenShift)
tektonTasksBundle, err := tekton_bundle.ReadBundle([]string{tektonTasksBundlePath})
if err != nil {
return err
}
Expand Down
44 changes: 17 additions & 27 deletions data/olm-catalog/ssp-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -83,33 +83,6 @@ spec:
- list
- update
- watch
- apiGroups:
- '*'
resources:
- configmaps
verbs:
- create
- delete
- list
- watch
- apiGroups:
- '*'
resources:
- persistentvolumeclaims
verbs:
- '*'
- apiGroups:
- '*'
resources:
- pods
verbs:
- create
- apiGroups:
- '*'
resources:
- secrets
verbs:
- '*'
- apiGroups:
- admissionregistration.k8s.io
resources:
Expand Down Expand Up @@ -211,6 +184,15 @@ spec:
- infrastructures
verbs:
- get
- apiGroups:
- ""
resources:
- configmaps
verbs:
- create
- delete
- list
- watch
- apiGroups:
- ""
resources:
Expand Down Expand Up @@ -238,6 +220,7 @@ spec:
resources:
- persistentvolumeclaims
verbs:
- '*'
- create
- delete
- get
Expand Down Expand Up @@ -266,9 +249,16 @@ spec:
resources:
- pods
verbs:
- create
- get
- list
- watch
- apiGroups:
- ""
resources:
- secrets
verbs:
- '*'
- apiGroups:
- ""
resources:
Expand Down
29 changes: 29 additions & 0 deletions internal/common/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,3 +549,32 @@ func defaultStatusFunc(obj client.Object) ResourceStatus {
}
return status
}

// AppendDeepCopies appends deep copies of objects from a source slice to a destination slice.
// This function is useful for creating a new slice containing deep copies of the provided objects.
//
// The PT interface {*T ; client.Object } is a trick. It means that type PT satisfies
// an anonymous typeset defined directly in the function signature.
// This typeset interface { *T; client.Object } means that it is a pointer to T
// and implements interface client.Object.
//
// Parameters:
// - destination: The destination slice where the deep copies of objects will be appended.
// - objects: A slice of objects (of type T) to be deep copied and appended to the destination slice.
//
// Returns:
// - The updated destination slice containing the appended deep copies of objects.
//
// Type Parameters:
// - PT: A type that is a pointer to the object type (should implement *T and client.Object interfaces).
// - T: The actual object type.
func AppendDeepCopies[PT interface {
*T
client.Object
}, T any](destination []client.Object, objects []T) []client.Object {
for i := range objects {
var object = PT(&objects[i])
destination = append(destination, object.DeepCopyObject().(client.Object))
}
return destination
}
29 changes: 8 additions & 21 deletions internal/operands/tekton-pipelines/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
)

// +kubebuilder:rbac:groups=tekton.dev,resources=pipelines,verbs=list;watch;create;update;delete
// +kubebuilder:rbac:groups=*,resources=configmaps,verbs=list;watch;create;delete
// +kubebuilder:rbac:groups=core,resources=configmaps,verbs=list;watch;create;delete
Copy link
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity, is there any difference between using * or core? AFAIK, it will refer to the same apiGroup, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, * would grant permission to any API group in your cluster. Imagine that some custom API group, e.g. kubevirt.io would contain a custom ConfigMap resource that may be accessible (e.g. some ConfigMap that contains secrets).

I see that it's also possible to just specify an empty API group, as "" (groups="") and it would also refer to the core objects. Anyways, by using core we embrace least privilege principle and readability.

From the beginning I think that It was specified as * to speed up the development process (also some verbs) - because to have a specific value requires some effort.

Copy link
Member Author

@codingben codingben Aug 7, 2023

Choose a reason for hiding this comment

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

I love the explanation of that topic here [1]. Very good explanation and examples.

[1] https://learnk8s.io/rbac-kubernetes#modelling-access-to-resources

// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=list;watch;create;update;delete

const (
Expand Down Expand Up @@ -116,25 +116,15 @@ func (t *tektonPipelines) Reconcile(request *common.Request) ([]common.Reconcile

func (t *tektonPipelines) Cleanup(request *common.Request) ([]common.CleanupResult, error) {
var objects []client.Object

if request.CrdList.CrdExists(tektonCrd) {
for _, p := range t.pipelines {
o := p.DeepCopy()
objects = append(objects, o)
}
}
for _, cm := range t.configMaps {
o := cm.DeepCopy()
objects = append(objects, o)
}
for _, rb := range t.roleBindings {
o := rb.DeepCopy()
objects = append(objects, o)
}
for _, sa := range t.serviceAccounts {
o := sa.DeepCopy()
objects = append(objects, o)
objects = common.AppendDeepCopies(objects, t.pipelines)
}

objects = common.AppendDeepCopies(objects, t.configMaps)
objects = common.AppendDeepCopies(objects, t.roleBindings)
objects = common.AppendDeepCopies(objects, t.serviceAccounts)

namespace, isUserDefinedNamespace := getTektonPipelinesNamespace(request)
for i, o := range objects {
objectNamespace := namespace
Expand All @@ -144,10 +134,7 @@ func (t *tektonPipelines) Cleanup(request *common.Request) ([]common.CleanupResu
objects[i].SetNamespace(objectNamespace)
}

for _, cr := range t.clusterRoles {
o := cr.DeepCopy()
objects = append(objects, o)
}
objects = common.AppendDeepCopies(objects, t.clusterRoles)

return common.DeleteAll(request, objects...)
}
Expand Down
Loading
Loading