From 999496bf12573c9a0e069ba66abe023239e1cd3a Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Mon, 8 Jun 2026 09:53:33 -0400 Subject: [PATCH 1/2] OADP-6540: Skip restore of k8s RBAC system rolebindings PR #311 added skip logic for system rolebindings (system:image-pullers, system:image-builders, system:deployers) but only for the authorization.openshift.io API group. OpenShift stores these as rbac.authorization.k8s.io objects and exposes them via both API groups. Velero backs up both variants since rolebindings are not in Velero's cohabitating resources dedup list. During restore with namespace mapping, the k8s RBAC variants were restored with stale subject references (old namespace), overwriting the correct auto-created rolebindings and causing ErrImagePull. Add a new RestoreItemAction for rolebindings (rbac.authorization.k8s.io) that skips the same system rolebindings, letting OpenShift create them with correct namespace references. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy Signed-off-by: Tiger Kaovilai --- Makefile | 17 ++- README.md | 14 ++- velero-plugins/main.go | 5 + velero-plugins/rolebindings/k8s_restore.go | 41 +++++++ .../rolebindings/k8s_restore_test.go | 101 ++++++++++++++++++ .../{restore.go => ocp_restore.go} | 8 +- .../{restore_test.go => ocp_restore_test.go} | 0 7 files changed, 169 insertions(+), 17 deletions(-) create mode 100644 velero-plugins/rolebindings/k8s_restore.go create mode 100644 velero-plugins/rolebindings/k8s_restore_test.go rename velero-plugins/rolebindings/{restore.go => ocp_restore.go} (95%) rename velero-plugins/rolebindings/{restore_test.go => ocp_restore_test.go} (100%) diff --git a/Makefile b/Makefile index e8be7252..9cca82c2 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ BINS = $(wildcard velero-*) REPO ?= github.com/konveyor/openshift-velero-plugin -BUILD_IMAGE ?= openshift/origin-release:golang-1.14 +BUILD_IMAGE ?= golang:1.25 IMAGE ?= docker.io/konveyor/openshift-velero-plugin @@ -34,19 +34,16 @@ build-%: build: _output/$(BIN) _output/$(BIN): $(BIN)/*.go - mkdir -p .go/src/$(REPO) .go/pkg .go/.cache .go/std/$(ARCH) _output - cp -rp * .go/src/$(REPO) + mkdir -p .go/.cache _output docker run \ --rm \ - -v $$(pwd)/.go/pkg:/go/pkg:z \ - -v $$(pwd)/.go/src:/go/src:z \ - -v $$(pwd)/.go/std:/go/std:z \ + -v $$(pwd):/workspace:z \ -v $$(pwd)/.go/.cache:/go/.cache:z \ - -v $$(pwd)/_output:/go/src/$(REPO)/_output:z \ - -v $$(pwd)/.go/std/$(ARCH):/usr/local/go/pkg/linux_$(ARCH)_static:z \ - -w /go/src/$(REPO) \ + -w /workspace \ + -e GOCACHE=/go/.cache \ + -e GOFLAGS="-mod=mod -buildvcs=false" \ $(BUILD_IMAGE) \ - go build -installsuffix "static" -tags $(BUILDTAGS) -i -v -o _output/$(BIN) ./$(BIN) + go build -tags $(BUILDTAGS) -v -o _output/$(BIN) ./$(BIN) DOCKER_BUILD_ARGS ?= --platform=linux/amd64 ifneq ($(CLUSTER_OS),) diff --git a/README.md b/README.md index 48f24834..04e4cdc5 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,8 @@ Velero currently supports the following kinds of plugins: - Pod - Replica Set - Replication Controller -- Role Binding +- Role Binding (authorization.openshift.io) +- Role Binding (rbac.authorization.k8s.io) - Route - Security Context Constraints (SCC) - Secret @@ -545,9 +546,9 @@ time="2020-07-29T18:51:04Z" level=info msg="[pvc-restore] Returning pvc object a ### Role Binding -#### Restore Plugin +#### Restore Plugin (OpenShift authorization.openshift.io) -- **Resources**: rolebindings +- **Resources**: rolebinding.authorization.openshift.io - **Actions**: - Skips restore of system rolebindings ("system:image-pullers", "system:image-builders", "system:deployers") as these are automatically created by OpenShift - Updates namespaces in subjects when namespace mapping is enabled @@ -555,6 +556,13 @@ time="2020-07-29T18:51:04Z" level=info msg="[pvc-restore] Returning pvc object a - Preserves role references while updating namespace contexts - If restore namespace mapping is enabled, then the namespaces in RoleRef.Namespace, usernames, groupnames, and subjects are swapped accordingly +#### Restore Plugin (Kubernetes RBAC) + +- **Resources**: rolebindings (rbac.authorization.k8s.io) +- **Actions**: + - Skips restore of the same system rolebindings ("system:image-pullers", "system:image-builders", "system:deployers") for the Kubernetes RBAC API group + - OpenShift stores rolebindings in rbac.authorization.k8s.io format and exposes them via both API groups; both must be skipped during restore to avoid conflicts with auto-created rolebindings + ### Route #### Restore Plugin diff --git a/velero-plugins/main.go b/velero-plugins/main.go index c9d398df..8df60b3b 100644 --- a/velero-plugins/main.go +++ b/velero-plugins/main.go @@ -73,6 +73,7 @@ func main() { RegisterBackupItemAction("openshift.io/25-configmap-backup-plugin", newConfigMapBackupPlugin). RegisterRestoreItemAction("openshift.io/25-configmap-restore-plugin", newConfigMapRestorePlugin). RegisterRestoreItemAction("openshift.io/26-nonadmin-restore-plugin", newNonAdminRestorePlugin). + RegisterRestoreItemAction("openshift.io/27-rbac-role-bindings-restore-plugin", newRBACRoleBindingRestorePlugin). Serve() } @@ -235,3 +236,7 @@ func newConfigMapRestorePlugin(logger logrus.FieldLogger) (interface{}, error) { func newNonAdminRestorePlugin(logger logrus.FieldLogger) (interface{}, error) { return &nonadmin.RestorePluginNonAdmin{Log: logger}, nil } + +func newRBACRoleBindingRestorePlugin(logger logrus.FieldLogger) (interface{}, error) { + return &rolebindings.K8sRestorePlugin{Log: logger}, nil +} diff --git a/velero-plugins/rolebindings/k8s_restore.go b/velero-plugins/rolebindings/k8s_restore.go new file mode 100644 index 00000000..138bb452 --- /dev/null +++ b/velero-plugins/rolebindings/k8s_restore.go @@ -0,0 +1,41 @@ +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, _ := json.Marshal(input.Item) + json.Unmarshal(itemMarshal, &roleBinding) + + 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 +} diff --git a/velero-plugins/rolebindings/k8s_restore_test.go b/velero-plugins/rolebindings/k8s_restore_test.go new file mode 100644 index 00000000..cca27100 --- /dev/null +++ b/velero-plugins/rolebindings/k8s_restore_test.go @@ -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) + }) + } +} diff --git a/velero-plugins/rolebindings/restore.go b/velero-plugins/rolebindings/ocp_restore.go similarity index 95% rename from velero-plugins/rolebindings/restore.go rename to velero-plugins/rolebindings/ocp_restore.go index 9d483029..1ab94896 100644 --- a/velero-plugins/rolebindings/restore.go +++ b/velero-plugins/rolebindings/ocp_restore.go @@ -11,10 +11,10 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) -// systemRoleBindings contains rolebindings that are automatically created by OpenShift -// These rolebindings are expected to be created by the system and don't need restoring +// SystemRoleBindings contains rolebindings that are automatically created by OpenShift +// when a new project/namespace is created. These don't need restoring. // Reference: https://github.com/openshift/openshift-apiserver/blob/eefb161cffdc97a949d6e9cc81aa900005912a97/pkg/project/apiserver/registry/projectrequest/delegated/delegated.go#L111 -var systemRoleBindings = map[string]bool{ +var SystemRoleBindings = map[string]bool{ "system:image-pullers": true, "system:image-builders": true, "system:deployers": true, @@ -42,7 +42,7 @@ func (p *RestorePlugin) Execute(input *velero.RestoreItemActionExecuteInput) (*v p.Log.Infof("[rolebinding-restore] role binding - %s, API version", roleBinding.Name, roleBinding.APIVersion) - if systemRoleBindings[roleBinding.Name] { + if SystemRoleBindings[roleBinding.Name] { p.Log.Infof("[rolebinding-restore] Skipping system rolebinding %s as it will be automatically created", roleBinding.Name) return &velero.RestoreItemActionExecuteOutput{ SkipRestore: true, diff --git a/velero-plugins/rolebindings/restore_test.go b/velero-plugins/rolebindings/ocp_restore_test.go similarity index 100% rename from velero-plugins/rolebindings/restore_test.go rename to velero-plugins/rolebindings/ocp_restore_test.go From b3793d77bfc30e5d02e99cdb088026f557a8b6f6 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Mon, 8 Jun 2026 12:55:43 -0400 Subject: [PATCH 2/2] fix: handle json.Marshal/Unmarshal errors in k8s restore plugin Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy Signed-off-by: Tiger Kaovilai --- velero-plugins/rolebindings/k8s_restore.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/velero-plugins/rolebindings/k8s_restore.go b/velero-plugins/rolebindings/k8s_restore.go index 138bb452..a7c128f2 100644 --- a/velero-plugins/rolebindings/k8s_restore.go +++ b/velero-plugins/rolebindings/k8s_restore.go @@ -25,8 +25,13 @@ func (p *K8sRestorePlugin) Execute(input *velero.RestoreItemActionExecuteInput) p.Log.Info("[rbac-rolebinding-restore] Entering RBAC Role Bindings restore plugin") roleBinding := rbacv1.RoleBinding{} - itemMarshal, _ := json.Marshal(input.Item) - json.Unmarshal(itemMarshal, &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)