replace dots in shared-dir volume names

[deepsm007]

https://redhat-internal.slack.com/archives/CBN38N3MW/p1779090578809989

/cc @openshift/test-platform

Fix shared-dir volume naming for Kubernetes DNS compliance

This change makes multi-stage test pods generated by ci-operator produce Kubernetes-safe volume and mount names for Secret-backed shared directories when the secret name contains dots.

What changed

• Adds sharedDirVolumeName(secret) which normalizes a secret-derived volume/mount name by replacing '.' with '-' to make it DNS-1123 compliant (e.g., "test-4.22" → "test-4-22").
• Updates addSharedDirSecret to use the normalized name for the Pod volume Name and the container VolumeMount Name, while continuing to set SecretName to the original secret string (the referenced Secret resource is unchanged).
• Adds unit tests (TestAddSharedDirSecret) that verify volumes and mounts are created exactly once and validate naming behavior for names with and without dots and with existing dash-separated segments.

Component and behavioral impact

• Affects the multi-stage test executor (pkg/steps/multi_stage) used by ci-operator. Pods that mount Secret-backed shared directories will now always have valid volume and mount names, preventing Pod creation failures when secret names include dots (common in versioned names like "4.22").
• Backwards compatible: the Secret resource name used to look up the Secret is unchanged; only the in-Pod volume/mount identifiers are normalized.

Testing

• Table-driven unit tests cover cases ensuring:
  • Names without dots remain unchanged.
  • Names with dots are normalized by replacing dots with dashes.
  • SecretName and mount path remain correct.

This resolves pod creation errors caused by invalid volume names derived from dotted secret names while keeping Secret references intact.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 61df0536-6e9b-4cb3-8b3d-f77599dcc333

📥 Commits

Reviewing files that changed from the base of the PR and between e49fd05 and b03a0f7.

📒 Files selected for processing (2)

• pkg/steps/multi_stage/gen.go
• pkg/steps/multi_stage/gen_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• pkg/steps/multi_stage/gen.go
• pkg/steps/multi_stage/gen_test.go

---

📝 Walkthrough

Walkthrough

Normalize Kubernetes volume names for shared-dir secrets by replacing dots with dashes via sharedDirVolumeName(), update addSharedDirSecret to use the normalized name for the volume and mount (SecretName unchanged), and add a table-driven test validating the mutation and naming behavior.

Changes

Shared dir secret handling

Layer / File(s)	Summary
Volume name normalization and secret handling pkg/steps/multi_stage/gen.go	Adds sharedDirVolumeName() which replaces . with -; updates addSharedDirSecret() to use the derived name for the created Volume and container VolumeMount while keeping SecretName equal to the original secret; minor import formatting change.
Test validation for secret volume mounting pkg/steps/multi_stage/gen_test.go	Adds TestAddSharedDirSecret (table-driven) that asserts the pod gains exactly one secret-backed volume and matching volume mount, verifies dot-to-dash replacement in the derived names, and checks the mount path equals SecretMountPath.

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 11 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Go Error Handling	⚠️ Warning	Function addSharedDirSecret dereferences pod.Spec.Containers[0] without nil checks on pod or Containers slice length, violating the requirement to check nil before dereferencing.	Add defensive checks: verify pod != nil and len(pod.Spec.Containers) > 0 before accessing pod.Spec.Containers[0].
Test Structure And Quality	❓ Inconclusive	Custom check specifies reviewing Ginkgo test code (It blocks, BeforeEach/AfterEach). The test added is standard Go testing, not Ginkgo. No Ginkgo framework used in this package.	Check inapplicable. TestAddSharedDirSecret uses standard Go testing. Clarify if criteria apply to standard Go tests.

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: introducing a function to replace dots in shared-dir volume names and updating the volume/mount creation logic.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Test Coverage For New Features	✅ Passed	New pure function sharedDirVolumeName and modified addSharedDirSecret have comprehensive test coverage. TestAddSharedDirSecret is table-driven with 3 cases validating dot-to-dash conversion.
Stable And Deterministic Test Names	✅ Passed	Custom check is for Ginkgo test names (It, Describe, Context, When), but the PR adds only standard Go unit tests (func Test...) with static, deterministic test case names. Not applicable to this PR.
Microshift Test Compatibility	✅ Passed	The PR adds a standard Go unit test (TestAddSharedDirSecret), not Ginkgo e2e tests. The custom check applies only to "new Ginkgo e2e tests"; this unit test is not within scope.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	The PR adds unit tests in gen_test.go using standard Go testing package, not Ginkgo e2e tests. The custom check for SNO compatibility is for new Ginkgo e2e tests only, so it does not apply here.
Topology-Aware Scheduling Compatibility	✅ Passed	Change is a utility function for volume naming (replacing dots with hyphens), not a scheduling constraint. No affinity, topology spread, node selectors, or topology-dependent logic introduced.
Ote Binary Stdout Contract	✅ Passed	No OTE Binary Stdout Contract violations. Changes are utility library functions and test code with no process-level stdout writes. No klog, fmt.Print*, or log.Print* calls detected.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No Ginkgo e2e tests added. Only a standard Go unit test (TestAddSharedDirSecret) for internal volume naming logic. Check not applicable.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@pkg/steps/multi_stage/gen_test.go`:
- Around line 548-552: The test currently dereferences vol.Secret and accesses
pod.Spec.Containers[0].VolumeMounts[0] without guards; add explicit nil/length
checks before those assertions: ensure vol.Secret != nil (and fail with t.Fatalf
or t.Errorf + return if nil) before checking vol.Secret.SecretName, and ensure
len(pod.Spec.Containers) > 0 and len(pod.Spec.Containers[0].VolumeMounts) > 0
before accessing the first mount (again failing the test with a clear message if
missing) so the test reports diagnostic failures instead of panicking.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: c3312e3b-25dc-483c-99b7-53ae231cb1a5

📥 Commits

Reviewing files that changed from the base of the PR and between 5da1b49 and 021d7a3.

📒 Files selected for processing (2)

• pkg/steps/multi_stage/gen.go
• pkg/steps/multi_stage/gen_test.go

[eifrach]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deepsm007, eifrach

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [deepsm007]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[deepsm007]

/hold

[openshift-merge-bot]

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e

[openshift-ci]

@deepsm007: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[openshift-merge-bot]

Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage.