Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
7 changes: 7 additions & 0 deletions e2e-tests/framework/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ func (crb ClusterRoleBinding) AddSubject(k KubectlRunner, sa ServiceAccount) err
type ServiceAccount struct {
Name string
Namespace string
Label string
}

func (sa ServiceAccount) Create(k KubectlRunner) error {
Expand All @@ -132,6 +133,12 @@ func (sa ServiceAccount) Create(k KubectlRunner) error {
return fmt.Errorf("failed to create ServiceAccount %s in %s: %w", sa.Name, sa.Namespace, err)
}
log.Printf("created ServiceAccount %s in %s", sa.Name, sa.Namespace)
if sa.Label != "" {
_, err = k.Run("label", "serviceaccount", sa.Name, "-n", sa.Namespace, sa.Label)
if err != nil {
return fmt.Errorf("failed to label ServiceAccount %s: %w", sa.Name, err)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
}
Comment on lines +136 to +142

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add tests for optional ServiceAccount labeling.

Cover the empty-label path, successful label invocation, and propagated label failure. As per coding guidelines, all new features require tests.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e-tests/framework/resources.go` around lines 136 - 141, Add tests for the
ServiceAccount labeling logic in the resource creation flow around the sa.Label
branch: verify empty labels skip k.Run, non-empty labels invoke the expected
label command successfully, and label command errors are propagated with the
ServiceAccount context. Use the existing test patterns and helpers in the
resources tests.

Source: Coding guidelines

return nil
}

Expand Down
7 changes: 7 additions & 0 deletions e2e-tests/testdata/gadget_cr.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: crane-e2e.openshift.io/v1
kind: Gadget
metadata:
name: test-gadget
spec:
color: red
size: 3
Comment on lines +1 to +7

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Rename the new YAML fixtures consistently with the repository convention.

  • e2e-tests/testdata/gadget_cr.yaml#L1-L7: rename to Gadget_crane-e2e.openshift.io_v1_simple-nginx-nopv_test-gadget.yaml.
  • e2e-tests/testdata/gadget_crd.yaml#L1-L27: rename to CustomResourceDefinition_apiextensions.k8s.io_v1_clusterscoped_gadgets.crane-e2e.openshift.io.yaml.
  • e2e-tests/tests/tier0/ca10_crd_flags_test.go#L120-L123: update both ReadTestdataFile arguments.

As per coding guidelines, YAML resource names should follow Kind_group_version_namespace_name.yaml.

📍 Affects 3 files
  • e2e-tests/testdata/gadget_cr.yaml#L1-L7 (this comment)
  • e2e-tests/testdata/gadget_crd.yaml#L1-L27
  • e2e-tests/tests/tier0/ca10_crd_flags_test.go#L120-L123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e-tests/testdata/gadget_cr.yaml` around lines 1 - 7, Rename
e2e-tests/testdata/gadget_cr.yaml to
Gadget_crane-e2e.openshift.io_v1_simple-nginx-nopv_test-gadget.yaml and
e2e-tests/testdata/gadget_crd.yaml to
CustomResourceDefinition_apiextensions.k8s.io_v1_clusterscoped_gadgets.crane-e2e.openshift.io.yaml;
update both ReadTestdataFile arguments in ca10_crd_flags_test.go to reference
the new filenames.

Source: Coding guidelines

27 changes: 27 additions & 0 deletions e2e-tests/testdata/gadget_crd.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: gadgets.crane-e2e.openshift.io
spec:
group: crane-e2e.openshift.io
names:
kind: Gadget
listKind: GadgetList
plural: gadgets
singular: gadget
scope: Namespaced
versions:
- name: v1
served: true
storage: true
schema:
openAPIV3Schema:
type: object
properties:
spec:
type: object
properties:
color:
type: string
size:
type: integer
217 changes: 217 additions & 0 deletions e2e-tests/tests/tier0/ca10_crd_flags_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
package e2e

import (
"log"
"path/filepath"

"github.com/konveyor/crane/e2e-tests/config"
. "github.com/konveyor/crane/e2e-tests/framework"
"github.com/konveyor/crane/e2e-tests/utils"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("CRD group filtering during export", func() {
appName := "simple-nginx-nopv"
namespace := "simple-nginx-nopv"
serviceName := "my-" + appName
It("[CA-10a] Should skip CRD when --crd-skip-group matches", Label("tier0"), func() {
scenario := NewMigrationScenario(
appName,
namespace,
config.K8sDeployBin,
config.CraneBin,
config.SourceContext,
config.TargetContext,
)
srcApp := scenario.SrcApp
tgtApp := scenario.TgtApp
kubectlSrc := scenario.KubectlSrc
kubectlTgt := scenario.KubectlTgt
runner := scenario.Crane

paths, err := NewScenarioPaths("crane-ca10a-*")
Expect(err).NotTo(HaveOccurred())
crdYAML, err := utils.ReadTestdataFile("widget_crd.yaml")
Expect(err).NotTo(HaveOccurred())
crYAML, err := utils.ReadTestdataFile("widget_cr.yaml")
Expect(err).NotTo(HaveOccurred())

crd := CustomResourceDefinition{
Name: "widgets.crane-e2e.example.com",
YAML: crdYAML,
}

cr := CustomResource{
Name: "test-widget",
Namespace: namespace,
Kind: "Widget",
YAML: crYAML,
Resource: "widgets",
}
excludedResource := []utils.ResourceMatch{
{Kind: "CustomResourceDefinition", Name: crd.Name},
}
includedResource := []utils.ResourceMatch{
{Kind: cr.Kind, Name: cr.Name, Scope: namespace},
}
exportOpts := ExportOptions{Namespace: srcApp.Namespace, ExportDir: paths.ExportDir,
CRDSkipGroups: []string{"crane-e2e.example.com"}}
transformOpts := TransformOptions{ExportDir: paths.ExportDir, TransformDir: paths.TransformDir}
applyOpts := ApplyOptions{ExportDir: paths.ExportDir, TransformDir: paths.TransformDir,
OutputDir: paths.OutputDir}

DeferCleanup(func() {
if err := ResourceCleanup([]KubectlRunner{kubectlSrc, kubectlTgt}, []Resource{cr, crd}); err != nil {
log.Printf("Resources cleanup: %v", err)
}
if err := CleanupScenario(paths.TempDir, srcApp, tgtApp); err != nil {
log.Printf("Scenario cleanup: %v", err)
}
})

By("Deploying app on source cluster")
Expect(PrepareSourceApp(srcApp, kubectlSrc)).NotTo(HaveOccurred())

By("Creating Widget CRD on source")
Expect(crd.Create(kubectlSrc)).NotTo(HaveOccurred())

By("Waiting for CRD to be established")
Expect(crd.WaitForEstablished(kubectlSrc)).NotTo(HaveOccurred())

By("Creating Widget custom resource in namespace")
Expect(cr.Create(kubectlSrc)).NotTo(HaveOccurred())

By("Waiting for source pods and endpoints to drain")
WaitForSourceQuiesce(kubectlSrc, namespace, "app="+appName, serviceName)

By("Running crane export with --crd-skip-group, transform, apply")
Expect(RunCranePipelineWithChecks(runner, exportOpts, transformOpts, applyOpts)).NotTo(HaveOccurred())

By("Verifying CRD is excluded from export")
found, err := utils.AssertResourcesExist(paths.ExportDir, excludedResource)
Expect(err).NotTo(HaveOccurred())
Expect(found).To(BeFalse())

By("Verifying Widget CR exists in namespace export directory")
nameSpaceDir := filepath.Join(paths.ExportDir, "resources", namespace)
found, err = utils.AssertResourcesExist(nameSpaceDir, includedResource)
Expect(err).NotTo(HaveOccurred())
Expect(found).To(BeTrue())
})

It("[CA-10b] Should include CRD when --crd-include-group matches", Label("tier0"), func() {
scenario := NewMigrationScenario(
appName,
namespace,
config.K8sDeployBin,
config.CraneBin,
config.SourceContext,
config.TargetContext,
)
srcApp := scenario.SrcApp
tgtApp := scenario.TgtApp
kubectlSrc := scenario.KubectlSrc
kubectlTgt := scenario.KubectlTgt
runner := scenario.Crane

paths, err := NewScenarioPaths("crane-ca10b-*")
Expect(err).NotTo(HaveOccurred())
crdYAML, err := utils.ReadTestdataFile("gadget_crd.yaml")
Expect(err).NotTo(HaveOccurred())
crYAML, err := utils.ReadTestdataFile("gadget_cr.yaml")
Expect(err).NotTo(HaveOccurred())

crd := CustomResourceDefinition{
Name: "gadgets.crane-e2e.openshift.io",
YAML: crdYAML,
}
cr := CustomResource{
Name: "test-gadget",
Namespace: namespace,
Kind: "Gadget",
YAML: crYAML,
Resource: "gadgets",
}
tgtNameSpace := Namespace{Name: namespace}

exportOpts := ExportOptions{Namespace: srcApp.Namespace, ExportDir: paths.ExportDir,
CRDIncludeGroups: []string{"crane-e2e.openshift.io"}}
transformOpts := TransformOptions{ExportDir: paths.ExportDir, TransformDir: paths.TransformDir}
applyOpts := ApplyOptions{ExportDir: paths.ExportDir, TransformDir: paths.TransformDir,
OutputDir: paths.OutputDir}

DeferCleanup(func() {
if err := ResourceCleanup([]KubectlRunner{kubectlSrc, kubectlTgt}, []Resource{cr, crd, tgtNameSpace}); err != nil {
log.Printf("Resources cleanup: %v", err)
}
if err := CleanupScenario(paths.TempDir, srcApp, tgtApp); err != nil {
log.Printf("Scenario cleanup: %v", err)
}
})

By("Deploying app on source cluster")
Expect(PrepareSourceApp(srcApp, kubectlSrc)).NotTo(HaveOccurred())

By("Creating Gadget CRD on source")
Expect(crd.Create(kubectlSrc)).NotTo(HaveOccurred())

By("Waiting for CRD to be established")
Expect(crd.WaitForEstablished(kubectlSrc)).NotTo(HaveOccurred())

By("Creating Gadget custom resource in namespace")
Expect(cr.Create(kubectlSrc)).NotTo(HaveOccurred())

By("Waiting for source pods and endpoints to drain")
WaitForSourceQuiesce(kubectlSrc, namespace, "app="+appName, serviceName)

By("Running crane export with --crd-include-group, transform, apply")
Expect(RunCranePipelineWithChecks(runner, exportOpts, transformOpts, applyOpts)).NotTo(HaveOccurred())

By("Verifying CRD exists in export _cluster directory")
exportClusterPath := filepath.Join(paths.ExportDir, "resources", namespace, "_cluster")
found, err := utils.AssertResourcesExist(exportClusterPath, []utils.ResourceMatch{
{Kind: "CustomResourceDefinition", Name: crd.Name}})
Expect(err).NotTo(HaveOccurred())
Expect(found).To(BeTrue())

By("Verifying Gadget CR exists in namespace export directory")
namespaceDir := filepath.Join(paths.ExportDir, "resources", namespace)
found, err = utils.AssertResourcesExist(namespaceDir, []utils.ResourceMatch{
{Kind: cr.Kind, Name: cr.Name, Scope: namespace}})
Expect(err).NotTo(HaveOccurred())
Expect(found).To(BeTrue())

// By("Verifying CRD exists in output _cluster directory")
// outputClusterPath := filepath.Join(paths.OutputDir, "resources", "_cluster")
// Expect(ValidateDirResources(outputClusterPath, crdPatterns)).NotTo(HaveOccurred())

By("Creating namespace on target cluster")
Expect(tgtNameSpace.Create(kubectlTgt)).NotTo(HaveOccurred())

By("Applying cluster resources to target")
Expect(kubectlTgt.ApplyDir(filepath.Join(paths.OutputDir, "resources", "_cluster"))).NotTo(HaveOccurred())

By("Waiting for CRD to be established on target")
Expect(crd.WaitForEstablished(kubectlTgt)).NotTo(HaveOccurred())

By("Applying namespace resources to target")
Expect(kubectlTgt.ApplyDir(filepath.Join(paths.OutputDir, "resources", namespace))).NotTo(HaveOccurred())

By("Verifying Gadget CR exists on target")
_, err = kubectlTgt.Run("get", "gadget", cr.Name, "-n", namespace)
Expect(err).NotTo(HaveOccurred())

By("Verifying Gadget CR has correct spec values on target")
color, err := kubectlTgt.Run("get", "gadget", cr.Name, "-n", namespace,
"-o", "jsonpath={.spec.color}")
Expect(err).NotTo(HaveOccurred())
Expect(color).To(Equal("red"))

By("Scaling target deployment and validating app")
Expect(kubectlTgt.ScaleDeployment(namespace, appName, 1)).NotTo(HaveOccurred())
Eventually(tgtApp.Validate, "5m", "10s").Should(Succeed())

})

})
110 changes: 110 additions & 0 deletions e2e-tests/tests/tier0/ca8_label_scoped_export_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
package e2e

import (
"log"
"path/filepath"

"github.com/konveyor/crane/e2e-tests/config"
. "github.com/konveyor/crane/e2e-tests/framework"
"github.com/konveyor/crane/e2e-tests/utils"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var _ = Describe("Cluster-level export filtering", func() {
It("[CA-8] Should export only labeled workload and its RBAC with --label-selector", Label("tier0"), func() {
appName := "simple-nginx-nopv"
namespace := "simple-nginx-nopv"
serviceName := "my-" + appName

scenario := NewMigrationScenario(
appName,
namespace,
config.K8sDeployBin,
config.CraneBin,
config.SourceContext,
config.TargetContext,
)
srcApp := scenario.SrcApp
tgtApp := scenario.TgtApp
kubectlSrc := scenario.KubectlSrc
kubectlTgt := scenario.KubectlTgt
runner := scenario.Crane
paths, err := NewScenarioPaths("crane-ca8-*")
Expect(err).NotTo(HaveOccurred())

exportOpts := ExportOptions{Namespace: srcApp.Namespace, ExportDir: paths.ExportDir,
LabelSelector: "app=" + appName}
transformOpts := TransformOptions{ExportDir: paths.ExportDir, TransformDir: paths.TransformDir}
applyOpts := ApplyOptions{ExportDir: paths.ExportDir, TransformDir: paths.TransformDir,
OutputDir: paths.OutputDir}

inScopeSA := ServiceAccount{Name: "nginx-sa", Namespace: namespace, Label: "app=simple-nginx-nopv"}
outOfScopeSA := ServiceAccount{Name: "out-of-scope-sa", Namespace: namespace, Label: "app=outScopedApp"}

inScopeCR := ClusterRole{Name: "in-scope-cr", Verb: "get,list,watch", Resource: "pods", Label: "app=" + appName}
outOfScopeCR := ClusterRole{Name: "out-scope-cr", Verb: "get,list,watch,create,update,delete", Resource: "pods", Label: "app=outScopedApp"}

inScopeSubject := "--serviceaccount=" + namespace + ":" + inScopeSA.Name
outScopeSubject := "--serviceaccount=" + namespace + ":" + outOfScopeSA.Name

inScopeBinding := ClusterRoleBinding{Name: "in-scope-crb", ClusterRoleName: inScopeCR.Name, Subject: inScopeSubject, Label: "app=" + appName}
outOfScopeBinding := ClusterRoleBinding{Name: "out-scope-crb", ClusterRoleName: outOfScopeCR.Name, Subject: outScopeSubject, Label: "app=outScopedApp"}

outOfScopeResources := []utils.ResourceMatch{
{Kind: "ClusterRoleBinding", Name: outOfScopeBinding.Name},
{Kind: "ClusterRole", Name: outOfScopeCR.Name},
}
inScopeResources := []utils.ResourceMatch{
{Kind: "ClusterRoleBinding", Name: inScopeBinding.Name},
{Kind: "ClusterRole", Name: inScopeCR.Name},
}
DeferCleanup(func() {
if err := ResourceCleanup([]KubectlRunner{kubectlSrc, kubectlTgt}, []Resource{
inScopeBinding, outOfScopeBinding, inScopeCR, outOfScopeCR, inScopeSA, outOfScopeSA}); err != nil {
log.Printf("Resources cleanup: %v", err)
}
if err := CleanupScenario(paths.TempDir, srcApp, tgtApp); err != nil {
log.Printf("Scenario cleanup: %v", err)
}
})

By("Deploying app on source cluster")
Expect(PrepareSourceApp(srcApp, kubectlSrc)).NotTo(HaveOccurred())

By("Creating in-scope ServiceAccount with matching label")
Expect(inScopeSA.Create(kubectlSrc)).NotTo(HaveOccurred())

By("Creating out-of-scope ServiceAccount with different label")
Expect(outOfScopeSA.Create(kubectlSrc)).NotTo(HaveOccurred())

By("Creating in-scope ClusterRole with matching label")
Expect(inScopeCR.Create(kubectlSrc)).NotTo(HaveOccurred())

By("Creating out-of-scope ClusterRole with different label")
Expect(outOfScopeCR.Create(kubectlSrc)).NotTo(HaveOccurred())

By("Creating in-scope ClusterRoleBinding with matching label")
Expect(inScopeBinding.Create(kubectlSrc)).NotTo(HaveOccurred())

By("Creating out-of-scope ClusterRoleBinding with different label")
Expect(outOfScopeBinding.Create(kubectlSrc)).NotTo(HaveOccurred())

By("Waiting for source pods and endpoints to drain")
WaitForSourceQuiesce(kubectlSrc, namespace, "app="+appName, serviceName)

By("Running crane export with label-selector, transform, apply")
Expect(RunCranePipelineWithChecks(runner, exportOpts, transformOpts, applyOpts)).NotTo(HaveOccurred())

By("Verifying out-of-scope resources are not in export _cluster directory")
exportClusterPath := filepath.Join(paths.ExportDir, "resources", namespace, "_cluster")
found, err := utils.AssertResourcesExist(exportClusterPath, outOfScopeResources)
Expect(err).NotTo(HaveOccurred())
Expect(found).To(BeFalse())

By("Verifying in-scope ClusterRole and ClusterRoleBinding exist in export, transform, and output")
found, err = utils.AssertResourcesExist(exportClusterPath, inScopeResources)
Expect(err).NotTo(HaveOccurred())
Expect(found).To(BeTrue())
Comment thread
RanWurmbrand marked this conversation as resolved.
Outdated
})
})
Loading
Loading