Skip to content
Open
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
255 changes: 224 additions & 31 deletions test/extended/storage/storage_networkpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package storage

import (
"context"
"encoding/json"
"fmt"
"strings"

"github.com/blang/semver/v4"
g "github.com/onsi/ginkgo/v2"
o "github.com/onsi/gomega"
exutil "github.com/openshift/origin/test/extended/util"
Expand All @@ -13,14 +15,42 @@ import (
e2e "k8s.io/kubernetes/test/e2e/framework"
)

// ResourceType defines the type of Kubernetes workload resource
// storage_networkpolicy.go contains tests for verifying network policy configurations
// for storage-related operators in OpenShift.
//
// This test suite validates that storage operators and their workload resources have
// the correct network policy labels to ensure proper network segmentation and security.
//
// Test Coverage:
// 1. CSO (Cluster Storage Operator) resources - verifies network policy labels on
// cluster-storage-operator, csi-snapshot-controller, and related deployments
// 2. CSI driver resources - verifies network policy labels on platform-specific CSI
// drivers (AWS EBS/EFS, Azure Disk/File, GCP PD/Filestore, vSphere, IBM Cloud,
// OpenStack Cinder/Manila, SMB)
// 3. LSO (Local Storage Operator) resources - verifies network policy labels on
// related deployment and diskmaker daemonsets
// 4. NetworkPolicy resources - ensures required NetworkPolicies exist with correct
// PodSelector labels for CSO, CSI, and LSO namespaces
//
// LSO-specific Design Considerations:
// - LSONamespace is defined as a variable (not constant) because LSO can be installed
// in a user-specified namespace, allowing for customization based on actual deployment

// ResourceType defines the type of Openshift workload resource
type ResourceType string

const (
ResourceTypeDeployment ResourceType = "Deployment"
ResourceTypeDaemonSet ResourceType = "DaemonSet"
)

// lsoInfo contains information about the Local Storage Operator installation
type lsoInfo struct {
Installed bool
Namespace string
Version string
}

// resourceCheck defines a check for a workload resource (Deployment, DaemonSet, etc.)
type resourceCheck struct {
ResourceType ResourceType
Expand All @@ -37,6 +67,11 @@ var (
npLabelOperatorMetricsRange = map[string]string{"openshift.storage.network-policy.operator-metrics-range": "allow"}
npLabelMetricsRange = map[string]string{"openshift.storage.network-policy.metrics-range": "allow"}
npLabelAllEgress = map[string]string{"openshift.storage.network-policy.all-egress": "allow"}
// LSO specific network policy labels
npLabelLSOAPIServer = map[string]string{"openshift.storage.network-policy.lso.api-server": "allow"}
npLabelLSODNS = map[string]string{"openshift.storage.network-policy.lso.dns": "allow"}
npLabelLSOOperatorMetrics = map[string]string{"openshift.storage.network-policy.lso.operator-metrics": "allow"}
npLabelLSODiskmakerMetrics = map[string]string{"openshift.storage.network-policy.lso.diskmaker-metrics": "allow"}
)

func mergeLabels(maps ...map[string]string) map[string]string {
Expand All @@ -58,6 +93,9 @@ var (
csiOperatorWithAllEgressRequiredLabels = mergeLabels(npLabelAPI, npLabelDNS, npLabelOperatorMetricsRange, npLabelAllEgress)
csiControllerRequiredLabels = mergeLabels(npLabelAPI, npLabelDNS, npLabelMetricsRange)
csiControllerWithAllEgressRequiredLabels = mergeLabels(npLabelAPI, npLabelDNS, npLabelMetricsRange, npLabelAllEgress)
// LSO specific required labels
lsoOperatorRequiredLabels = mergeLabels(npLabelLSOAPIServer, npLabelLSODNS, npLabelLSOOperatorMetrics)
lsoDiskmakerRequiredLabels = mergeLabels(npLabelLSOAPIServer, npLabelLSODNS, npLabelLSODiskmakerMetrics)
)

type npCheck struct {
Expand Down Expand Up @@ -101,11 +139,26 @@ var networkPolicyChecks = []npCheck{
// },
}

// getLSONetworkPolicyCheck returns the LSO network policy check configuration
// based on the detected LSO installation information
func getLSONetworkPolicyCheck(lso *lsoInfo) npCheck {
return npCheck{
Namespace: lso.Namespace,
RequiredPodSelectors: []map[string]string{
npLabelLSOAPIServer,
npLabelLSODNS,
npLabelLSOOperatorMetrics,
npLabelLSODiskmakerMetrics,
},
}
}

var _ = g.Describe("[sig-storage][OCPFeature:StorageNetworkPolicy] Storage Network Policy", func() {
defer g.GinkgoRecover()
var (
oc = exutil.NewCLI("storage-network-policy")
currentPlatform = e2e.TestContext.Provider
lsoInstallInfo *lsoInfo // LSO installation information detected once per suite
)

g.BeforeEach(func() {
Expand All @@ -114,6 +167,20 @@ var _ = g.Describe("[sig-storage][OCPFeature:StorageNetworkPolicy] Storage Netwo
if isMicroShift {
g.Skip("Storage Network Policy tests are not supported on MicroShift")
}

// Detect LSO installation only once (cache the result)
if lsoInstallInfo == nil {
lsoInstallInfo, err = getLSOInfo(oc)
o.Expect(err).NotTo(o.HaveOccurred(), "Failed to detect LSO installation")

if lsoInstallInfo.Installed {
supported := isLSOVersionSupported(lsoInstallInfo.Version)
g.By(fmt.Sprintf("Detected LSO installed in namespace: %s, version: %s (network policy support: %v)",
lsoInstallInfo.Namespace, lsoInstallInfo.Version, supported))
} else {
g.By("LSO is not installed on this cluster")
}
}
})

g.It("should verify required labels for CSO related Operators", func() {
Expand Down Expand Up @@ -296,6 +363,55 @@ var _ = g.Describe("[sig-storage][OCPFeature:StorageNetworkPolicy] Storage Netwo
runResourceChecks(oc, CSIResourcesToCheck, currentPlatform)
})

g.It("should verify required labels for LSO related resources", func() {
// Skip if LSO is not installed or version is lower than 4.21.0
if !lsoInstallInfo.Installed {
g.Skip("LSO is not installed on this cluster")
} else if !isLSOVersionSupported(lsoInstallInfo.Version) {
g.Skip(fmt.Sprintf("LSO network policy support requires version >= 4.21.0, current version: %s", lsoInstallInfo.Version))
}
Comment on lines +368 to +372
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if !lsoInstallInfo.Installed {
g.Skip("LSO is not installed on this cluster")
} else if !isLSOVersionSupported(lsoInstallInfo.Version) {
g.Skip(fmt.Sprintf("LSO network policy support requires version >= 4.21.0, current version: %s", lsoInstallInfo.Version))
}
if !lsoInstallInfo.Installed {
g.Skip("LSO is not installed on this cluster")
}
if !isLSOVersionSupported(lsoInstallInfo.Version) {
g.Skip(fmt.Sprintf("LSO network policy support requires version >= 4.21.0, current version: %s", lsoInstallInfo.Version))
}

will be more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, it looks clear, will update later.


LSOResourcesToCheck := []resourceCheck{
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could also consider just append it to CSIResourcesToCheck if lso is installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment. I once considered combining, but it will give us the clear result from the case level (skipped if LSO is not installed), so I think we can keep this :)

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, we could keep it, from the reports scope makes sense.

{
ResourceType: ResourceTypeDeployment,
Namespace: lsoInstallInfo.Namespace,
Name: "local-storage-operator",
Platform: "all",
RequiredLabels: lsoOperatorRequiredLabels,
},
{
ResourceType: ResourceTypeDaemonSet,
Namespace: lsoInstallInfo.Namespace,
Name: "diskmaker-manager",
Platform: "all",
RequiredLabels: lsoDiskmakerRequiredLabels,
},
{
ResourceType: ResourceTypeDaemonSet,
Namespace: lsoInstallInfo.Namespace,
Name: "diskmaker-discovery",
Platform: "all",
RequiredLabels: lsoDiskmakerRequiredLabels,
},
}

runResourceChecks(oc, LSOResourcesToCheck, currentPlatform)
})

g.It("should ensure required NetworkPolicies exist with correct labels for LSO", func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

The same above, we could just append it in g.It("should ensure required NetworkPolicies exist with correct labels"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same above :)

// Skip if LSO is not installed or version is lower than 4.21.0
if !lsoInstallInfo.Installed {
g.Skip("LSO is not installed on this cluster")
} else if !isLSOVersionSupported(lsoInstallInfo.Version) {
g.Skip(fmt.Sprintf("LSO network policy support requires version >= 4.21.0, current version: %s", lsoInstallInfo.Version))
}

// Get LSO network policy check configuration
lsoCheck := getLSONetworkPolicyCheck(lsoInstallInfo)

verifyNetworkPolicyPodSelectors(oc, lsoCheck.Namespace, lsoCheck.RequiredPodSelectors)
})

g.It("should ensure required NetworkPolicies exist with correct labels", func() {
for _, c := range networkPolicyChecks {
_, err := oc.AdminKubeClient().CoreV1().Namespaces().Get(context.TODO(), c.Namespace, metav1.GetOptions{})
Expand All @@ -310,36 +426,7 @@ var _ = g.Describe("[sig-storage][OCPFeature:StorageNetworkPolicy] Storage Netwo
g.Fail(fmt.Sprintf("Error fetching namespace %s: %v", c.Namespace, err))
}

// List all NetworkPolicies in the namespace
npList, err := oc.AdminKubeClient().NetworkingV1().NetworkPolicies(c.Namespace).List(context.TODO(), metav1.ListOptions{})
o.Expect(err).NotTo(o.HaveOccurred(), fmt.Sprintf("failed to list NetworkPolicies in namespace %s", c.Namespace))

// For each required PodSelector, verify that at least one NetworkPolicy has it
for _, requiredSelector := range c.RequiredPodSelectors {
found := false
var matchedNPName string

for _, np := range npList.Items {
if podSelectorContainsLabels(np.Spec.PodSelector.MatchLabels, requiredSelector) {
found = true
matchedNPName = np.Name
break
}
}

// Format the required selector for error messages
labelPairs := []string{}
for k, v := range requiredSelector {
labelPairs = append(labelPairs, fmt.Sprintf("%s=%s", k, v))
}
selectorDesc := strings.Join(labelPairs, ", ")

if found {
g.By(fmt.Sprintf("Found NetworkPolicy %s/%s with required PodSelector labels: %s", c.Namespace, matchedNPName, selectorDesc))
} else {
o.Expect(found).To(o.BeTrue(), fmt.Sprintf("No NetworkPolicy in namespace %s has PodSelector with labels: %s", c.Namespace, selectorDesc))
}
}
verifyNetworkPolicyPodSelectors(oc, c.Namespace, c.RequiredPodSelectors)
}
})
})
Expand Down Expand Up @@ -404,6 +491,78 @@ func runResourceChecks(oc *exutil.CLI, resources []resourceCheck, currentPlatfor
}
}

// isLSOVersionSupported checks if the LSO version is 4.21.0 or higher
// Supported version formats: "4.21.0", "4.21.0-202511252120"
func isLSOVersionSupported(versionStr string) bool {
// Minimum required version for LSO network policy support
minVersion := semver.MustParse("4.21.0")

// Parse the LSO version
// The version string may contain build metadata (e.g., "4.21.0-202511252120")
// semver.Parse handles this correctly
version, err := semver.Parse(versionStr)
if err != nil {
e2e.Logf("Failed to parse LSO version %q: %v", versionStr, err)
return false
}

// Compare versions: returns true if version >= minVersion
return version.GTE(minVersion)
}

// getLSOInfo detects if LSO is installed by searching for local-storage-operator CSV
// across all namespaces and returns its namespace and version information
func getLSOInfo(oc *exutil.CLI) (*lsoInfo, error) {
info := &lsoInfo{
Installed: false,
}

// Get all CSVs across all namespaces matching local-storage-operator pattern
// Command: oc get csv -A -o json
output, err := oc.AsAdmin().Run("get").Args("csv", "-A", "-o", "json").Output()
if err != nil {
return info, fmt.Errorf("failed to list ClusterServiceVersions: %v", err)
}

// Parse the JSON output to find local-storage-operator CSV
// The output contains a list of CSVs with metadata.name and metadata.namespace
var csvList struct {
Items []struct {
Metadata struct {
Name string `json:"name"`
Namespace string `json:"namespace"`
} `json:"metadata"`
Spec struct {
Version string `json:"version"`
} `json:"spec"`
Status struct {
Phase string `json:"phase"`
} `json:"status"`
} `json:"items"`
}

if err := json.Unmarshal([]byte(output), &csvList); err != nil {
return info, fmt.Errorf("failed to parse CSV list: %v", err)
}

// Search for local-storage-operator CSV
for _, csv := range csvList.Items {
// Match CSV name pattern: local-storage-operator.*
if strings.HasPrefix(csv.Metadata.Name, "local-storage-operator") {
// Only consider CSVs in Succeeded phase
if csv.Status.Phase == "Succeeded" {
info.Installed = true
info.Namespace = csv.Metadata.Namespace
info.Version = csv.Spec.Version
return info, nil
}
}
}
Comment on lines +520 to +560
Copy link
Member

Choose a reason for hiding this comment

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

There may be a better way to do this. oc.AsAdmin().Run() spawns a new oc process, then there is a new custom struct to unmarshal the text output. You could instead use the API types directly from operator-framework here:
https://github.com/operator-framework/api/blob/ebdb4e0b321b668f7bc5146e2972b1ce4529d109/pkg/operators/v1alpha1/clusterserviceversion_types.go#L601-L619

oadp-operator for example imports the API here:
https://github.com/openshift/oadp-operator/blob/oadp-dev/must-gather/pkg/cli.go#L15
then adds it to the scheme here:
https://github.com/openshift/oadp-operator/blob/oadp-dev/must-gather/pkg/cli.go#L82

IMO, you should at least be able to use ClusterServiceVersionList from the operator-framework repo, instead of defining the new csvList struct here. Even better if you can add some new method to test/extended/util/client.go to get a client that supports ClusterServiceVersion so you could list CSV's the same way you list NetworkPolicies and avoid oc.AsAdmin().Run().


// LSO not found or not in Succeeded phase
return info, nil
}

// podSelectorContainsLabels checks if actualLabels contains all key-value pairs from requiredLabels
func podSelectorContainsLabels(actualLabels map[string]string, requiredLabels map[string]string) bool {
for key, value := range requiredLabels {
Expand All @@ -413,3 +572,37 @@ func podSelectorContainsLabels(actualLabels map[string]string, requiredLabels ma
}
return true
}

// verifyNetworkPolicyPodSelectors verifies that for each required PodSelector, at least one NetworkPolicy has it
func verifyNetworkPolicyPodSelectors(oc *exutil.CLI, namespace string, requiredPodSelectors []map[string]string) {
// List all NetworkPolicies in the namespace
npList, err := oc.AdminKubeClient().NetworkingV1().NetworkPolicies(namespace).List(context.TODO(), metav1.ListOptions{})
o.Expect(err).NotTo(o.HaveOccurred(), fmt.Sprintf("failed to list NetworkPolicies in namespace %s", namespace))

// For each required PodSelector, verify that at least one NetworkPolicy has it
for _, requiredSelector := range requiredPodSelectors {
found := false
var matchedNPName string

for _, np := range npList.Items {
if podSelectorContainsLabels(np.Spec.PodSelector.MatchLabels, requiredSelector) {
found = true
matchedNPName = np.Name
break
}
}

// Format the required selector for error messages
labelPairs := []string{}
for k, v := range requiredSelector {
labelPairs = append(labelPairs, fmt.Sprintf("%s=%s", k, v))
}
selectorDesc := strings.Join(labelPairs, ", ")

if found {
g.By(fmt.Sprintf("Found NetworkPolicy %s/%s with required PodSelector labels: %s", namespace, matchedNPName, selectorDesc))
} else {
o.Expect(found).To(o.BeTrue(), fmt.Sprintf("No NetworkPolicy in namespace %s has PodSelector with labels: %s", namespace, selectorDesc))
}
}
}