From 6bb5e4a7e873009788318ee6ad8a7d249c387985 Mon Sep 17 00:00:00 2001 From: Rafaela Soares Date: Mon, 9 Mar 2026 17:01:20 +0000 Subject: [PATCH 1/3] enable CodeRabbit Multi-Repo Analysis --- .coderabbit.yaml | 13 ++++++ .github/workflows/test-with-coverage.yml | 2 +- .github/workflows/upload-coverage.yml | 2 +- README.adoc | 2 +- codecov.yaml | 4 +- go.mod | 6 +-- go.sum | 8 ++-- pkg/owners/fetcher.go | 44 ++++++++++++------- pkg/owners/fetcher_test.go | 12 +++-- pkg/test/config/toolchainconfig.go | 25 ----------- pkg/test/masteruserrecord/masteruserrecord.go | 7 +++ 11 files changed, 68 insertions(+), 57 deletions(-) create mode 100644 .coderabbit.yaml diff --git a/.coderabbit.yaml b/.coderabbit.yaml new file mode 100644 index 00000000..24eed590 --- /dev/null +++ b/.coderabbit.yaml @@ -0,0 +1,13 @@ +inheritance: true +knowledge_base: + linked_repositories: + - repository: 'codeready-toolchain/api' + instructions: > + api contains the shared API types consumed by toolchain-common. + When reviewing toolchain-common changes: + - Check if toolchain-common is using deprecated or outdated fields or types from api - suggest migrating to the recommended alternatives. + - Verify assumptions — if toolchain-common expects specific constants (condition types, reasons, labels) or type structures from api, check if those match the actual api definitions. + - If toolchain-common duplicates constants or type definitions that already exist in api, suggest using the api definitions instead. + - Flag if toolchain-common is making assumptions about api's internal state or behavior that isn't documented or guaranteed. + - Check if changes in toolchain-common (especially in pkg/cluster, pkg/condition, pkg/configuration, pkg/status) align with api patterns and conventions. + \ No newline at end of file diff --git a/.github/workflows/test-with-coverage.yml b/.github/workflows/test-with-coverage.yml index 990b1449..a9f6cdb1 100644 --- a/.github/workflows/test-with-coverage.yml +++ b/.github/workflows/test-with-coverage.yml @@ -28,7 +28,7 @@ jobs: make test-with-coverage - name: Upload coverage artifact - uses: actions/upload-artifact@v6 + uses: actions/upload-artifact@v7 with: name: coverage path: ./out/coverage/coverage.txt diff --git a/.github/workflows/upload-coverage.yml b/.github/workflows/upload-coverage.yml index f3936656..e15806f4 100644 --- a/.github/workflows/upload-coverage.yml +++ b/.github/workflows/upload-coverage.yml @@ -12,7 +12,7 @@ jobs: steps: - name: Download coverage artifact - uses: actions/download-artifact@v7 + uses: actions/download-artifact@v8 with: name: coverage github-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/README.adoc b/README.adoc index 5cd7ab0d..ea2d3d4b 100644 --- a/README.adoc +++ b/README.adoc @@ -8,7 +8,7 @@ This repo is for controllers, libs, scripts, make files, etc to be shared betwee == Build -Requires Go version 1.24.x (1.24.11 or higher) - download for your development environment https://golang.org/dl/[here]. +Requires Go version 1.24.x (1.24.13 or higher) - download for your development environment https://golang.org/dl/[here]. This repository uses https://github.com/golang/go/wiki/Modules[Go modules]. diff --git a/codecov.yaml b/codecov.yaml index 1ff17b98..fa0a0e02 100644 --- a/codecov.yaml +++ b/codecov.yaml @@ -10,12 +10,12 @@ coverage: # solid green color. range: "20...60" -# See https://docs.codecov.com/docs/commit-status + # See https://docs.codecov.com/docs/commit-status status: # project will give us the diff in the total code coverage between a commit # and its parent project: - default: + default: # Allow the coverage to drop by 1% and posting a success status. threshold: 1% # Patch gives just the coverage of the patch diff --git a/go.mod b/go.mod index 6ad6d17c..a1db1d16 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/codeready-toolchain/toolchain-common go 1.24.4 -toolchain go1.24.11 +toolchain go1.24.13 require ( github.com/go-logr/logr v1.4.2 @@ -26,7 +26,7 @@ require ( ) require ( - github.com/codeready-toolchain/api v0.0.0-20260108115150-4c6695ed18de + github.com/codeready-toolchain/api v0.0.0-20260305144020-4ff0e6b6e174 github.com/ghodss/yaml v1.0.0 github.com/google/go-cmp v0.7.0 github.com/google/go-github/v52 v52.0.0 @@ -49,7 +49,7 @@ require ( github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/cloudflare/circl v1.6.1 // indirect + github.com/cloudflare/circl v1.6.3 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect diff --git a/go.sum b/go.sum index ea50ad67..271c51f1 100644 --- a/go.sum +++ b/go.sum @@ -18,10 +18,10 @@ github.com/bwesterb/go-ristretto v1.2.0/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7N github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cloudflare/circl v1.1.0/go.mod h1:prBCrKB9DV4poKZY1l9zBXg2QJY7mvgRvtMxxK7fi4I= -github.com/cloudflare/circl v1.6.1 h1:zqIqSPIndyBh1bjLVVDHMPpVKqp8Su/V+6MeDzzQBQ0= -github.com/cloudflare/circl v1.6.1/go.mod h1:uddAzsPgqdMAYatqJ0lsjX1oECcQLIlRpzZh3pJrofs= -github.com/codeready-toolchain/api v0.0.0-20260108115150-4c6695ed18de h1:rc39ZRUk2AyADzH+bt5On7V7qJz6syUrI4qNyCSXY3o= -github.com/codeready-toolchain/api v0.0.0-20260108115150-4c6695ed18de/go.mod h1:O/HyAcz6fynt3LXr8wTGaIXt+0v6BpkiJvDxNQDCqyA= +github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8= +github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= +github.com/codeready-toolchain/api v0.0.0-20260305144020-4ff0e6b6e174 h1:hed3ZyardxswS6yMB0ME9l3vEkO+pFouyk4dvIiAQOo= +github.com/codeready-toolchain/api v0.0.0-20260305144020-4ff0e6b6e174/go.mod h1:PMg6kNHuCGNlu3MOdrCisqGkBpvzB0qS1+E6nrXxPAc= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/pkg/owners/fetcher.go b/pkg/owners/fetcher.go index b89983ba..6926ce3e 100644 --- a/pkg/owners/fetcher.go +++ b/pkg/owners/fetcher.go @@ -72,14 +72,22 @@ func (o *OwnerFetcher) GetOwners(ctx context.Context, obj metav1.Object) ([]*Obj return nil, nil // No owner } // Get the GVR for the owner - gvr, err := gvrForKind(ownerReference.Kind, ownerReference.APIVersion, o.resourceLists) + gvr, namespaced, err := gvrForKind(ownerReference.Kind, ownerReference.APIVersion, o.resourceLists) if err != nil { return nil, err } - // Get the owner object - ownerObject, err := o.dynamicClient.Resource(*gvr).Namespace(obj.GetNamespace()).Get(ctx, ownerReference.Name, metav1.GetOptions{}) + // Get the owner object; use namespace only for namespaced resources + resourceClient := o.dynamicClient.Resource(*gvr) + var ownerObject *unstructured.Unstructured + nsdName := ownerReference.Name + if namespaced { + ownerObject, err = resourceClient.Namespace(obj.GetNamespace()).Get(ctx, ownerReference.Name, metav1.GetOptions{}) + nsdName = fmt.Sprintf("%s/%s", obj.GetNamespace(), ownerReference.Name) + } else { + ownerObject, err = resourceClient.Get(ctx, ownerReference.Name, metav1.GetOptions{}) + } if err != nil { - return nil, err + return nil, fmt.Errorf("failed to fetch owner object %s %s : %w", nsdName, gvr.String(), err) } owner := &ObjectWithGVR{ Object: ownerObject, @@ -93,24 +101,26 @@ func (o *OwnerFetcher) GetOwners(ctx context.Context, obj metav1.Object) ([]*Obj return append(ownerOwners, owner), nil } -// gvrForKind returns GVR for the kind, if it's found in the available API list in the cluster -// returns an error if not found or failed to parse the API version -func gvrForKind(kind, apiVersion string, resourceLists []*metav1.APIResourceList) (*schema.GroupVersionResource, error) { - gvr, err := findGVRForKind(kind, apiVersion, resourceLists) +// gvrForKind returns GVR and whether the resource is namespaced for the kind, +// if it's found in the available API list in the cluster. +// Returns an error if not found or failed to parse the API version. +func gvrForKind(kind, apiVersion string, resourceLists []*metav1.APIResourceList) (*schema.GroupVersionResource, bool, error) { + gvr, namespaced, err := findGVRForKind(kind, apiVersion, resourceLists) if gvr == nil && err == nil { - return nil, fmt.Errorf("no resource found for kind %s in %s", kind, apiVersion) + return nil, false, fmt.Errorf("no resource found for kind %s in %s", kind, apiVersion) } - return gvr, err + return gvr, namespaced, err } -// findGVRForKind returns GVR for the kind, if it's found in the available API list in the cluster -// if not found then returns nil, nil -// returns nil, error if failed to parse the API version -func findGVRForKind(kind, apiVersion string, resourceLists []*metav1.APIResourceList) (*schema.GroupVersionResource, error) { +// findGVRForKind returns GVR and whether the resource is namespaced for the kind, +// if it's found in the available API list in the cluster. +// If not found then returns nil, false, nil. +// Returns nil, false, error if failed to parse the API version. +func findGVRForKind(kind, apiVersion string, resourceLists []*metav1.APIResourceList) (*schema.GroupVersionResource, bool, error) { // Parse the group and version from the APIVersion (e.g., "apps/v1" -> group: "apps", version: "v1") gv, err := schema.ParseGroupVersion(apiVersion) if err != nil { - return nil, fmt.Errorf("failed to parse APIVersion %s: %w", apiVersion, err) + return nil, false, fmt.Errorf("failed to parse APIVersion %s: %w", apiVersion, err) } // Look for a matching resource @@ -123,11 +133,11 @@ func findGVRForKind(kind, apiVersion string, resourceLists []*metav1.APIResource Group: gv.Group, Version: gv.Version, Resource: apiResource.Name, - }, nil + }, apiResource.Namespaced, nil } } } } - return nil, nil + return nil, false, nil } diff --git a/pkg/owners/fetcher_test.go b/pkg/owners/fetcher_test.go index 718cc789..961008f8 100644 --- a/pkg/owners/fetcher_test.go +++ b/pkg/owners/fetcher_test.go @@ -36,6 +36,7 @@ func TestGetOwners(t *testing.T) { replica := &appsv1.ReplicaSet{ObjectMeta: metav1.ObjectMeta{Name: "test-replica", Namespace: "test-namespace"}} deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "test-deployment", Namespace: "test-namespace"}} vm, vmi := newVMResources(t, "test-vm", "test-namespace") + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "test-node"}} testCases := map[string]struct { expectedOwners []client.Object @@ -49,6 +50,9 @@ func TestGetOwners(t *testing.T) { "with deployment & replica as owners": { expectedOwners: []client.Object{deployment, replica}, }, + "with cluster-scoped owner": { + expectedOwners: []client.Object{node, replica}, + }, "with vm, vmi, deployment & replica as owners": { expectedOwners: []client.Object{vm, vmi, deployment, replica}, }, @@ -162,7 +166,8 @@ func TestGetOwnersFailures(t *testing.T) { require.ErrorContains(t, err, inaccessibleResource) assert.True(t, apierrors.IsNotFound(err)) } else { - require.EqualError(t, err, "some error") + require.ErrorContains(t, err, "failed to fetch owner object") + require.ErrorContains(t, err, "some error") } require.Nil(t, owners) }) @@ -192,7 +197,8 @@ func TestGetOwnersFailures(t *testing.T) { require.ErrorContains(t, err, inaccessibleResource) assert.True(t, apierrors.IsNotFound(err)) } else { - require.EqualError(t, err, "some error") + require.ErrorContains(t, err, "failed to fetch owner object") + require.ErrorContains(t, err, "some error") } require.Len(t, owners, 1) }) @@ -315,7 +321,7 @@ func apiSchemeResourceList(t *testing.T) []*metav1.APIResourceList { resources = append(resources, &metav1.APIResourceList{ GroupVersion: gvk.GroupVersion().String(), APIResources: []metav1.APIResource{ - {Name: resource.Resource, Namespaced: true, Kind: gvk.Kind}, + {Name: resource.Resource, Namespaced: gvk.Kind != "Node", Kind: gvk.Kind}, }, }) } diff --git a/pkg/test/config/toolchainconfig.go b/pkg/test/config/toolchainconfig.go index 4f7b8c10..3dedea02 100644 --- a/pkg/test/config/toolchainconfig.go +++ b/pkg/test/config/toolchainconfig.go @@ -145,24 +145,6 @@ func (o DeactivationOption) UserSignupUnverifiedRetentionDays(value int) Deactiv return o } -type MetricsOption struct { - *ToolchainConfigOptionImpl -} - -func Metrics() *MetricsOption { - o := &MetricsOption{ - ToolchainConfigOptionImpl: &ToolchainConfigOptionImpl{}, - } - return o -} - -func (o MetricsOption) ForceSynchronization(value bool) MetricsOption { - o.addFunction(func(config *toolchainv1alpha1.ToolchainConfig) { - config.Spec.Host.Metrics.ForceSynchronization = &value - }) - return o -} - type NotificationsOption struct { *ToolchainConfigOptionImpl } @@ -586,13 +568,6 @@ func (o TiersOption) DefaultSpaceTier(value string) TiersOption { return o } -func (o TiersOption) DurationBeforeChangeTierRequestDeletion(value string) TiersOption { - o.addFunction(func(config *toolchainv1alpha1.ToolchainConfig) { - config.Spec.Host.Tiers.DurationBeforeChangeTierRequestDeletion = &value - }) - return o -} - func (o TiersOption) FeatureToggle(name string, weight *uint) TiersOption { o.addFunction(func(config *toolchainv1alpha1.ToolchainConfig) { if config.Spec.Host.Tiers.FeatureToggles == nil { diff --git a/pkg/test/masteruserrecord/masteruserrecord.go b/pkg/test/masteruserrecord/masteruserrecord.go index 0a99f1c0..a2037403 100644 --- a/pkg/test/masteruserrecord/masteruserrecord.go +++ b/pkg/test/masteruserrecord/masteruserrecord.go @@ -100,6 +100,13 @@ func Sub(sub string) MurModifier { } } +func Email(email string) MurModifier { + return func(mur *toolchainv1alpha1.MasterUserRecord) error { + mur.Spec.PropagatedClaims.Email = email + return nil + } +} + func StatusCondition(con toolchainv1alpha1.Condition) MurModifier { return func(mur *toolchainv1alpha1.MasterUserRecord) error { mur.Status.Conditions, _ = condition.AddOrUpdateStatusConditions(mur.Status.Conditions, con) From 016fe7f56eae514f000eaf5ba796dcbba8101005 Mon Sep 17 00:00:00 2001 From: Rafaela Soares Date: Mon, 9 Mar 2026 17:21:49 +0000 Subject: [PATCH 2/3] improve --- .coderabbit.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 24eed590..d7ce3fee 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -4,6 +4,7 @@ knowledge_base: - repository: 'codeready-toolchain/api' instructions: > api contains the shared API types consumed by toolchain-common. + When reviewing toolchain-common changes: - Check if toolchain-common is using deprecated or outdated fields or types from api - suggest migrating to the recommended alternatives. - Verify assumptions — if toolchain-common expects specific constants (condition types, reasons, labels) or type structures from api, check if those match the actual api definitions. From eb1f21cfc44b31f9c1fd16db21981705f123beb9 Mon Sep 17 00:00:00 2001 From: Rafaela Soares Date: Mon, 9 Mar 2026 17:22:13 +0000 Subject: [PATCH 3/3] clean --- .coderabbit.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.coderabbit.yaml b/.coderabbit.yaml index d7ce3fee..05a548ce 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -5,7 +5,7 @@ knowledge_base: instructions: > api contains the shared API types consumed by toolchain-common. - When reviewing toolchain-common changes: + When reviewing toolchain-common changes: - Check if toolchain-common is using deprecated or outdated fields or types from api - suggest migrating to the recommended alternatives. - Verify assumptions — if toolchain-common expects specific constants (condition types, reasons, labels) or type structures from api, check if those match the actual api definitions. - If toolchain-common duplicates constants or type definitions that already exist in api, suggest using the api definitions instead.