OADP-6540: Skip restore of k8s RBAC system rolebindings #402
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package rolebindings | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
|
|
||
| "github.com/sirupsen/logrus" | ||
| "github.com/vmware-tanzu/velero/pkg/plugin/velero" | ||
| rbacv1 "k8s.io/api/rbac/v1" | ||
| ) | ||
|
|
||
| // K8sRestorePlugin is a restore item action plugin for k8s RBAC rolebindings | ||
| type K8sRestorePlugin struct { | ||
| Log logrus.FieldLogger | ||
| } | ||
|
|
||
| // AppliesTo returns a velero.ResourceSelector that applies to k8s RBAC rolebindings | ||
| func (p *K8sRestorePlugin) AppliesTo() (velero.ResourceSelector, error) { | ||
| return velero.ResourceSelector{ | ||
| IncludedResources: []string{"rolebindings"}, | ||
| }, nil | ||
| } | ||
|
|
||
| // Execute skips system rolebindings that OpenShift auto-creates for new namespaces | ||
| func (p *K8sRestorePlugin) Execute(input *velero.RestoreItemActionExecuteInput) (*velero.RestoreItemActionExecuteOutput, error) { | ||
| p.Log.Info("[rbac-rolebinding-restore] Entering RBAC Role Bindings restore plugin") | ||
|
|
||
| roleBinding := rbacv1.RoleBinding{} | ||
| itemMarshal, err := json.Marshal(input.Item) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if err = json.Unmarshal(itemMarshal, &roleBinding); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| p.Log.Infof("[rbac-rolebinding-restore] role binding - %s, API version %s", roleBinding.Name, roleBinding.APIVersion) | ||
|
|
||
| if SystemRoleBindings[roleBinding.Name] { | ||
| p.Log.Infof("[rbac-rolebinding-restore] Skipping system rolebinding %s as it will be automatically created", roleBinding.Name) | ||
| return &velero.RestoreItemActionExecuteOutput{ | ||
| SkipRestore: true, | ||
| }, nil | ||
| } | ||
|
|
||
| return velero.NewRestoreItemActionExecuteOutput(input.Item), nil | ||
| } | ||
|
Comment on lines
+24
to
+46
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Check for namespace mapping patterns in rolebinding restore plugins
echo "=== Checking if authorization.openshift.io rolebindings are also backed up as rbac.authorization.k8s.io ==="
rg -n -C3 'authorization\.openshift\.io' velero-plugins/rolebindings/
echo -e "\n=== Checking NamespaceMapping usage in restore plugins ==="
rg -n 'NamespaceMapping' velero-plugins/ --type go -A3 -B3
echo -e "\n=== Checking AppliesTo resource selectors for rolebindings ==="
ast-grep --pattern $'func ($) AppliesTo() $_ {
$$$
IncludedResources: $$$
$$$
}'Repository: openshift/openshift-velero-plugin Length of output: 15391 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== rolebindings directory ==="
ls -la velero-plugins/rolebindings
echo -e "\n=== AppliesTo implementations in rolebindings plugins ==="
rg -n "func \\(p \\*.*\\) AppliesTo\\(" velero-plugins/rolebindings/*.go
rg -n "IncludedResources" velero-plugins/rolebindings/*.go
echo -e "\n=== NamespaceMapping usage in rolebindings plugins ==="
rg -n "NamespaceMapping" velero-plugins/rolebindings/*.go
echo -e "\n=== k8s_restore.go Execute() details around unmarshalling/skip logic ==="
sed -n '1,120p' velero-plugins/rolebindings/k8s_restore.go
echo -e "\n=== ocp_restore.go Execute() details around namespace mapping ==="
sed -n '1,160p' velero-plugins/rolebindings/ocp_restore.go
echo -e "\n=== k8s_restore_test.go (look for NamespaceMapping / subject mapping behavior) ==="
sed -n '1,220p' velero-plugins/rolebindings/k8s_restore_test.goRepository: openshift/openshift-velero-plugin Length of output: 10563 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== clusterrolebindings/restore.go (namespace mapping logic) ==="
sed -n '1,220p' velero-plugins/clusterrolebindings/restore.goRepository: openshift/openshift-velero-plugin Length of output: 2255 Add
🤖 Prompt for AI Agents
Member
Author
There was a problem hiding this comment. Not needed — Velero's built-in For system rolebindings, we skip them entirely ( The embedded namespace in Note Responses generated with Claude There was a problem hiding this comment.
✏️ Learnings added
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| package rolebindings | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/konveyor/openshift-velero-plugin/velero-plugins/util/test" | ||
| "github.com/sirupsen/logrus" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" | ||
| "github.com/vmware-tanzu/velero/pkg/plugin/velero" | ||
| rbacv1 "k8s.io/api/rbac/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| ) | ||
|
|
||
| func TestK8sRestorePluginAppliesTo(t *testing.T) { | ||
| restorePlugin := &K8sRestorePlugin{Log: test.NewLogger()} | ||
|
|
||
| expectedResources := []string{"rolebindings"} | ||
|
|
||
| selectedResources, err := restorePlugin.AppliesTo() | ||
| require.NoError(t, err) | ||
|
|
||
| assert.Equal(t, expectedResources, selectedResources.IncludedResources) | ||
| } | ||
|
|
||
| func TestK8sExecuteSystemRoleBindings(t *testing.T) { | ||
| restorePlugin := &K8sRestorePlugin{Log: logrus.New()} | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| rbName string | ||
| shouldSkip bool | ||
| }{ | ||
| { | ||
| name: "Skip system:image-pullers", | ||
| rbName: "system:image-pullers", | ||
| shouldSkip: true, | ||
| }, | ||
| { | ||
| name: "Skip system:image-builders", | ||
| rbName: "system:image-builders", | ||
| shouldSkip: true, | ||
| }, | ||
| { | ||
| name: "Skip system:deployers", | ||
| rbName: "system:deployers", | ||
| shouldSkip: true, | ||
| }, | ||
| { | ||
| name: "Don't skip regular rolebinding", | ||
| rbName: "my-custom-rolebinding", | ||
| shouldSkip: false, | ||
| }, | ||
| { | ||
| name: "Don't skip rolebinding with system: prefix but not in list", | ||
| rbName: "system:custom-role", | ||
| shouldSkip: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| roleBinding := rbacv1.RoleBinding{ | ||
| TypeMeta: metav1.TypeMeta{ | ||
| APIVersion: "rbac.authorization.k8s.io/v1", | ||
| Kind: "RoleBinding", | ||
| }, | ||
| ObjectMeta: metav1.ObjectMeta{ | ||
| Name: tt.rbName, | ||
| Namespace: "test-namespace", | ||
| }, | ||
| RoleRef: rbacv1.RoleRef{ | ||
| APIGroup: "rbac.authorization.k8s.io", | ||
| Kind: "ClusterRole", | ||
| Name: "test-role", | ||
| }, | ||
| } | ||
|
|
||
| unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&roleBinding) | ||
| require.NoError(t, err) | ||
|
|
||
| item := &unstructured.Unstructured{Object: unstructuredObj} | ||
|
|
||
| input := &velero.RestoreItemActionExecuteInput{ | ||
| Item: item, | ||
| Restore: &velerov1.Restore{ | ||
| Spec: velerov1.RestoreSpec{ | ||
| NamespaceMapping: map[string]string{}, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| output, err := restorePlugin.Execute(input) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, tt.shouldSkip, output.SkipRestore) | ||
| }) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
File renamed without changes.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🧩 Analysis chain
🌐 Web query:
Does the officialgolang:1.25Docker image publish multi-architecture manifests, and whenGOOS/GOARCHare unset doesgo builddefault to the container's OS/architecture?💡 Result:
The official golang:1.25 Docker image publishes multi-architecture manifests [1][2]. These manifests allow the Docker engine to automatically pull the appropriate image for the host's architecture (such as amd64, arm64v8, or others) [1][3]. When GOOS and GOARCH are unset, the go build command defaults to the OS and architecture of the environment where the command is executed [4][5]. In the context of a running Docker container, this corresponds to the OS and architecture of that container [3][5]. The Go toolchain determines these default values based on its own build configuration and the system's runtime environment [4][6][7].
Citations:
🏁 Script executed:
Repository: openshift/openshift-velero-plugin
Length of output: 1062
🏁 Script executed:
Repository: openshift/openshift-velero-plugin
Length of output: 3840
🏁 Script executed:
Repository: openshift/openshift-velero-plugin
Length of output: 1302
🏁 Script executed:
Repository: openshift/openshift-velero-plugin
Length of output: 157
🏁 Script executed:
Repository: openshift/openshift-velero-plugin
Length of output: 3171
🏁 Script executed:
Repository: openshift/openshift-velero-plugin
Length of output: 825
🏁 Script executed:
Repository: openshift/openshift-velero-plugin
Length of output: 105
🏁 Script executed:
Repository: openshift/openshift-velero-plugin
Length of output: 20451
Make
make buildhonor$(ARCH)when usinggolang:1.25golang:1.25is multi-arch, and thedocker runused for_output/$(BIN)doesn’t set--platform,GOOS, orGOARCH, sogo builddefaults to the container/host architecture; meanwhileARCH ?= amd64is unused in that recipe. This can produce_output/$(BIN)for an architecture different from what you intend.Suggested fix
🤖 Prompt for AI Agents