-
Notifications
You must be signed in to change notification settings - Fork 427
Fixing oc client cases for microshift and metal ipv6 profliles #2192
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds comprehensive end-to-end test infrastructure for OpenShift CLI, including extensive test suites with multiple orchestrated scenarios, test utility functions and types supporting CLI command execution and Kubernetes operations, a fixture management system for embedded test data, and numerous YAML manifests defining test resources such as custom resource definitions, pods, deployments, and cluster configurations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
/payload 4.22 nightly blocking |
|
@gangwgr: trigger 14 job(s) of type blocking for the nightly release of OCP 4.22
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ac0a49c0-fdc7-11f0-88f6-772b4a411e87-0 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: gangwgr The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@test/e2e/cli.go`:
- Around line 546-553: The code incorrectly treats the variable output as a file
path by running exec.Command("bash", "-c", "cat "+output+" | awk '{print $1}'");
instead, treat output (the byte/string from oc get events) as the data to parse:
remove the exec.Command call and compute result by splitting string(output) into
lines, extracting the first whitespace-separated field of each non-empty line
(e.g., using strings.Split and strings.Fields), joining or scanning those first
fields into a single result string, then run regexp.MatchString("unknown",
result) as before; update references to output and result accordingly.
In `@test/e2e/util.go`:
- Around line 847-853: The function getShouldPruneRSFromCreateTime can panic
when keepNum > totalCompletedRsListNum; add a bounds check to compute a safe end
index (e.g., end := totalCompletedRsListNum - keepNum; if end < 0 { end = 0 })
before slicing, then slice using rsList := totalCompletedRsList[0:end], and only
call sort.Strings and log when end > 0; return the resulting rsList (which will
be an empty slice when keepNum >= totalCompletedRsListNum).
- Around line 913-930: getLatestPayload currently calls http.Get without a
timeout and defers res.Body.Close() after ioutil.ReadAll, risking hangs and
leaking the response body on read errors; replace the call with an http.Client
that has a sensible Timeout (e.g., few seconds) and check err before using res,
then immediately defer res.Body.Close() right after verifying res != nil and err
== nil, and keep the existing error checks for ioutil.ReadAll/json.Unmarshal;
update references inside getLatestPayload accordingly.
In `@test/testdata/oc_cli/case72217/cr-custom-72217.yaml`:
- Around line 1-6: The CR includes an undefined field spec.test which will be
pruned because the CRD crd-customtask-72217.yaml only defines spec.cronSpec,
spec.image, and spec.replicas; update the CR (cr-custom-72217.yaml) to only use
the defined fields or modify the CRD (crd-customtask-72217.yaml) to add
spec.test to the structural schema (or enable preserveUnknownFields) so the
field is accepted—also apply the same fix for cr-cat-72217.yaml and
cr-cron-72217.yaml where spec.test is present.
In `@test/testdata/oc_cli/initContainer.yaml`:
- Around line 8-20: Add a securityContext to both the initContainer "wait" and
the main container "hello-pod" to satisfy restricted PodSecurity/SCC rules: set
allowPrivilegeEscalation: false, ensure runAsNonRoot: true (and a non-root
runAsUser if needed), and enable readOnlyRootFilesystem where appropriate;
update the initContainers and containers entries to include these
securityContext fields so the pod is accepted under restricted policies and
matches other fixtures like debugpod_48681.yaml.
🧹 Nitpick comments (7)
test/testdata/fixtures.go (1)
117-128: ListFixtures silently ignores WalkDir errors.If
fs.WalkDirencounters an error, it's returned but never checked or logged after the function completes. This could hide issues during test debugging.♻️ Suggested fix
func ListFixtures() []string { var fixtures []string - fs.WalkDir(embeddedFixtures, ".", func(path string, d fs.DirEntry, err error) error { + err := fs.WalkDir(embeddedFixtures, ".", func(path string, d fs.DirEntry, err error) error { if err != nil { return err } if !d.IsDir() { fixtures = append(fixtures, path) } return nil }) + if err != nil { + panic(fmt.Sprintf("failed to list fixtures: %v", err)) + } return fixtures }test/e2e/cli.go (1)
484-508: Unconventional assertion pattern may cause confusing test failures.Using
o.Expect(...).To(o.BeTrue())insideifconditions is unusual. If the expectation fails, Gomega will fail the test immediately regardless of theiflogic, making the control flow misleading.♻️ Suggested refactor
- if o.Expect(strings.Contains(architectureStr, "x86_64")).To(o.BeTrue()) { - if o.Expect(strings.Contains(imageInfo, "amd64")).To(o.BeTrue()) { + if strings.Contains(architectureStr, "x86_64") { + o.Expect(strings.Contains(imageInfo, "amd64")).To(o.BeTrue(), "Expected amd64 arch for x86_64 platform") e2e.Logf("Found the expected Arch amd64") - } else { - e2e.Failf("Failed to find the expected Arch for mirrored image") - } - } else if o.Expect(strings.Contains(architectureStr, "aarch64")).To(o.BeTrue()) { + } else if strings.Contains(architectureStr, "aarch64") { + o.Expect(strings.Contains(imageInfo, "arm64")).To(o.BeTrue(), "Expected arm64 arch for aarch64 platform") - if o.Expect(strings.Contains(imageInfo, "arm64")).To(o.BeTrue()) { e2e.Logf("Found the expected Arch aarch64") - } else { - e2e.Failf("Failed to find the expected Arch for mirrored image") - } } // ... similar pattern for other architecturestest/e2e/util.go (5)
11-11: Replace deprecatedio/ioutilpackage.
io/ioutilhas been deprecated since Go 1.16. The functions are now available inioandospackages. This affects multiple usages in this file (lines 918, 1058, 1087, 1151, 1324).Suggested replacements
- "io/ioutil"Then update usages:
ioutil.ReadAll→io.ReadAllioutil.ReadFile→os.ReadFileioutil.ReadDir→os.ReadDirioutil.WriteFile→os.WriteFile
622-630: Remove duplicategetRandomStringfunction.This function duplicates
GetRandomStringdefined at line 363. Use the exported version instead to avoid maintaining duplicate code.Suggested fix
-func getRandomString() string { - chars := "abcdefghijklmnopqrstuvwxyz0123456789" - seed := rand.New(rand.NewSource(time.Now().UnixNano())) - buffer := make([]byte, 8) - for index := range buffer { - buffer[index] = chars[seed.Intn(len(chars))] - } - return string(buffer) -}Replace any calls to
getRandomString()withGetRandomString().
477-488: Consider migrating from deprecatedwait.Polltowait.PollUntilContextTimeout.
wait.Pollis deprecated and doesn't support context cancellation. This pattern appears throughout the file. Line 933 already uses the newerwait.PollUntilContextTimeoutcorrectly - consider applying the same pattern to other poll usages for consistency and proper cancellation support.Example migration
- err := wait.Poll(5*time.Second, 5*time.Minute, func() (bool, error) { + err := wait.PollUntilContextTimeout(context.Background(), 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) { output, err := c.AsAdmin().WithoutNamespace().Run("get").Args("pod", podName, "-n", namespace, "-o=jsonpath={.status.conditions[?(@.type=='Ready')].status}").Output()
1047-1049: Simplify file reading - avoid unnecessary shell command.Using
exec.Command("bash", "-c", "cat ...")to read a file is unnecessarily complex whenos.ReadFilecan be used directly.Suggested fix
oauthFilePath := dirName + "/.dockerconfigjson" - secretContent, err := exec.Command("bash", "-c", fmt.Sprintf("cat %v", oauthFilePath)).Output() + secretContent, err := os.ReadFile(oauthFilePath) o.Expect(err).NotTo(o.HaveOccurred())
1322-1343: Temp file is never cleaned up.The temp file created at line 1323 is never deleted, which can cause temp directory to fill up during long test runs. Consider returning a cleanup function or using
os.CreateTempwith proper cleanup.Suggested approach
Either document that callers must clean up the returned file path, or use a pattern like:
// At the call site, ensure cleanup: tmpFile, err := applyResourceFromTemplate48681(oc, args...) if err == nil { defer os.Remove(tmpFile) }Or modify the function to accept a cleanup callback.
| if match, _ := regexp.MatchString("No resources found", string(output)); match { | ||
| e2e.Logf("No events in project: %v", projectN) | ||
| } else { | ||
| result, _ := exec.Command("bash", "-c", "cat "+output+" | awk '{print $1}'").Output() | ||
| if match, _ := regexp.MatchString("unknown", string(result)); match { | ||
| e2e.Failf("Does not show timestamp as expected: %v", result) | ||
| } | ||
| } |
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.
Bug: cat command used on event output string instead of file path.
The variable output contains the actual events text from oc get events, not a file path. The cat command will fail or produce incorrect results.
🐛 Suggested fix
if match, _ := regexp.MatchString("No resources found", string(output)); match {
e2e.Logf("No events in project: %v", projectN)
} else {
- result, _ := exec.Command("bash", "-c", "cat "+output+" | awk '{print $1}'").Output()
+ // Parse the output directly instead of using cat
+ lines := strings.Split(output, "\n")
+ for _, line := range lines {
+ fields := strings.Fields(line)
+ if len(fields) > 0 && fields[0] == "unknown" {
+ e2e.Failf("Does not show timestamp as expected: %v", line)
+ }
+ }
- if match, _ := regexp.MatchString("unknown", string(result)); match {
- e2e.Failf("Does not show timestamp as expected: %v", result)
- }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if match, _ := regexp.MatchString("No resources found", string(output)); match { | |
| e2e.Logf("No events in project: %v", projectN) | |
| } else { | |
| result, _ := exec.Command("bash", "-c", "cat "+output+" | awk '{print $1}'").Output() | |
| if match, _ := regexp.MatchString("unknown", string(result)); match { | |
| e2e.Failf("Does not show timestamp as expected: %v", result) | |
| } | |
| } | |
| if match, _ := regexp.MatchString("No resources found", string(output)); match { | |
| e2e.Logf("No events in project: %v", projectN) | |
| } else { | |
| // Parse the output directly instead of using cat | |
| lines := strings.Split(output, "\n") | |
| for _, line := range lines { | |
| fields := strings.Fields(line) | |
| if len(fields) > 0 && fields[0] == "unknown" { | |
| e2e.Failf("Does not show timestamp as expected: %v", line) | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@test/e2e/cli.go` around lines 546 - 553, The code incorrectly treats the
variable output as a file path by running exec.Command("bash", "-c", "cat
"+output+" | awk '{print $1}'"); instead, treat output (the byte/string from oc
get events) as the data to parse: remove the exec.Command call and compute
result by splitting string(output) into lines, extracting the first
whitespace-separated field of each non-empty line (e.g., using strings.Split and
strings.Fields), joining or scanning those first fields into a single result
string, then run regexp.MatchString("unknown", result) as before; update
references to output and result accordingly.
| func getShouldPruneRSFromCreateTime(totalCompletedRsList []string, totalCompletedRsListNum int, keepNum int) []string { | ||
| rsList := totalCompletedRsList[0:(totalCompletedRsListNum - keepNum)] | ||
| sort.Strings(rsList) | ||
| e2e.Logf("rsList %v:", rsList) | ||
| return rsList | ||
|
|
||
| } |
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.
Missing bounds check can cause panic.
If keepNum exceeds totalCompletedRsListNum, the slice operation [0:(totalCompletedRsListNum - keepNum)] will panic with a negative index.
Suggested fix
func getShouldPruneRSFromCreateTime(totalCompletedRsList []string, totalCompletedRsListNum int, keepNum int) []string {
+ if keepNum >= totalCompletedRsListNum {
+ return []string{}
+ }
rsList := totalCompletedRsList[0:(totalCompletedRsListNum - keepNum)]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func getShouldPruneRSFromCreateTime(totalCompletedRsList []string, totalCompletedRsListNum int, keepNum int) []string { | |
| rsList := totalCompletedRsList[0:(totalCompletedRsListNum - keepNum)] | |
| sort.Strings(rsList) | |
| e2e.Logf("rsList %v:", rsList) | |
| return rsList | |
| } | |
| func getShouldPruneRSFromCreateTime(totalCompletedRsList []string, totalCompletedRsListNum int, keepNum int) []string { | |
| if keepNum >= totalCompletedRsListNum { | |
| return []string{} | |
| } | |
| rsList := totalCompletedRsList[0:(totalCompletedRsListNum - keepNum)] | |
| sort.Strings(rsList) | |
| e2e.Logf("rsList %v:", rsList) | |
| return rsList | |
| } |
🤖 Prompt for AI Agents
In `@test/e2e/util.go` around lines 847 - 853, The function
getShouldPruneRSFromCreateTime can panic when keepNum > totalCompletedRsListNum;
add a bounds check to compute a safe end index (e.g., end :=
totalCompletedRsListNum - keepNum; if end < 0 { end = 0 }) before slicing, then
slice using rsList := totalCompletedRsList[0:end], and only call sort.Strings
and log when end > 0; return the resulting rsList (which will be an empty slice
when keepNum >= totalCompletedRsListNum).
| func getLatestPayload(url string) string { | ||
| res, err := http.Get(url) | ||
| if err != nil { | ||
| e2e.Failf("unable to get http with error: %v", err) | ||
| } | ||
| body, err := ioutil.ReadAll(res.Body) | ||
| defer res.Body.Close() | ||
| if err != nil { | ||
| e2e.Failf("unable to parse the http result with error: %v", err) | ||
| } | ||
|
|
||
| var data map[string]interface{} | ||
| if err := json.Unmarshal(body, &data); err != nil { | ||
| e2e.Failf("unable to parse JSON with error: %v", err) | ||
| } | ||
| pullSpec, _ := data["pullSpec"].(string) | ||
| return pullSpec | ||
| } |
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.
HTTP request lacks timeout and has potential resource leak.
http.Gethas no timeout and can hang tests indefinitely if the server doesn't respond.defer res.Body.Close()is placed afterioutil.ReadAll, so ifReadAllfails, the body won't be closed.
Suggested fix
func getLatestPayload(url string) string {
- res, err := http.Get(url)
+ client := &http.Client{Timeout: 30 * time.Second}
+ res, err := client.Get(url)
if err != nil {
e2e.Failf("unable to get http with error: %v", err)
}
- body, err := ioutil.ReadAll(res.Body)
defer res.Body.Close()
+ body, err := io.ReadAll(res.Body)
if err != nil {
e2e.Failf("unable to parse the http result with error: %v", err)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func getLatestPayload(url string) string { | |
| res, err := http.Get(url) | |
| if err != nil { | |
| e2e.Failf("unable to get http with error: %v", err) | |
| } | |
| body, err := ioutil.ReadAll(res.Body) | |
| defer res.Body.Close() | |
| if err != nil { | |
| e2e.Failf("unable to parse the http result with error: %v", err) | |
| } | |
| var data map[string]interface{} | |
| if err := json.Unmarshal(body, &data); err != nil { | |
| e2e.Failf("unable to parse JSON with error: %v", err) | |
| } | |
| pullSpec, _ := data["pullSpec"].(string) | |
| return pullSpec | |
| } | |
| func getLatestPayload(url string) string { | |
| client := &http.Client{Timeout: 30 * time.Second} | |
| res, err := client.Get(url) | |
| if err != nil { | |
| e2e.Failf("unable to get http with error: %v", err) | |
| } | |
| defer res.Body.Close() | |
| body, err := io.ReadAll(res.Body) | |
| if err != nil { | |
| e2e.Failf("unable to parse the http result with error: %v", err) | |
| } | |
| var data map[string]interface{} | |
| if err := json.Unmarshal(body, &data); err != nil { | |
| e2e.Failf("unable to parse JSON with error: %v", err) | |
| } | |
| pullSpec, _ := data["pullSpec"].(string) | |
| return pullSpec | |
| } |
🤖 Prompt for AI Agents
In `@test/e2e/util.go` around lines 913 - 930, getLatestPayload currently calls
http.Get without a timeout and defers res.Body.Close() after ioutil.ReadAll,
risking hangs and leaking the response body on read errors; replace the call
with an http.Client that has a sensible Timeout (e.g., few seconds) and check
err before using res, then immediately defer res.Body.Close() right after
verifying res != nil and err == nil, and keep the existing error checks for
ioutil.ReadAll/json.Unmarshal; update references inside getLatestPayload
accordingly.
| apiVersion: "example.com/v1" | ||
| kind: Customtask72217 | ||
| metadata: | ||
| name: my-new-customtask-object | ||
| spec: | ||
| test: test |
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
🏁 Script executed:
find test/testdata/oc_cli/case72217 -type f -name "*.yaml" -o -name "*.yml" | head -20Repository: openshift/oc
Length of output: 370
🏁 Script executed:
cat -n test/testdata/oc_cli/case72217/cr-custom-72217.yamlRepository: openshift/oc
Length of output: 210
🏁 Script executed:
cat -n test/testdata/oc_cli/case72217/crd-customtask-72217.yamlRepository: openshift/oc
Length of output: 940
🏁 Script executed:
grep -r "case72217\|cr-custom-72217\|crd-customtask-72217" test --include="*.go" --include="*.sh" | head -20Repository: openshift/oc
Length of output: 336
🏁 Script executed:
grep -B 10 -A 20 "case72217" test/e2e/cli.goRepository: openshift/oc
Length of output: 1841
🏁 Script executed:
grep -B 5 -A 40 "customTaskCRDF\|customCRF" test/e2e/cli.go | grep -A 40 "Create the customtask"Repository: openshift/oc
Length of output: 38
🏁 Script executed:
sed -n '1,5000p' test/e2e/cli.go | grep -B 3 -A 25 "customTaskCRDF :="Repository: openshift/oc
Length of output: 1830
🏁 Script executed:
sed -n '1,5000p' test/e2e/cli.go | grep -B 2 -A 10 "customCRF\|cr of.*custom"Repository: openshift/oc
Length of output: 1743
🏁 Script executed:
cat -n test/testdata/oc_cli/case72217/crd-customtask-72217.yaml | grep -i "preserve\|test"Repository: openshift/oc
Length of output: 38
🏁 Script executed:
cat test/testdata/oc_cli/case72217/crd-customtask-72217.yamlRepository: openshift/oc
Length of output: 730
🏁 Script executed:
git log --oneline --all -S "case72217" -- test/testdata/oc_cli/case72217 | head -5Repository: openshift/oc
Length of output: 38
🏁 Script executed:
git log --oneline --all -S "case72217" | head -10Repository: openshift/oc
Length of output: 142
🏁 Script executed:
grep -r "spec.test\|\.test\|test:" test/testdata/oc_cli/case72217/ --include="*.yaml" --include="*.yml"Repository: openshift/oc
Length of output: 245
🏁 Script executed:
grep -A 20 "waitCreateCr(oc, customCRF" test/e2e/cli.go | head -30Repository: openshift/oc
Length of output: 1502
CR uses a field not defined in its CRD schema.
Line 6 sets spec.test, but the CRD (crd-customtask-72217.yaml) only defines spec.cronSpec, spec.image, and spec.replicas. With v1 structural schemas and no preserveUnknownFields setting, unknown fields are pruned, so spec.test will be silently discarded. Align the CR with the schema or extend the CRD.
Note: The same issue affects cr-cat-72217.yaml and cr-cron-72217.yaml—all three CRs in this test case include spec.test which is undefined in their respective CRD schemas.
🔧 Example fix (extend CRD schema)
# test/testdata/oc_cli/case72217/crd-customtask-72217.yaml
properties:
spec:
type: object
properties:
cronSpec:
type: string
image:
type: string
replicas:
type: integer
+ test:
+ type: string🤖 Prompt for AI Agents
In `@test/testdata/oc_cli/case72217/cr-custom-72217.yaml` around lines 1 - 6, The
CR includes an undefined field spec.test which will be pruned because the CRD
crd-customtask-72217.yaml only defines spec.cronSpec, spec.image, and
spec.replicas; update the CR (cr-custom-72217.yaml) to only use the defined
fields or modify the CRD (crd-customtask-72217.yaml) to add spec.test to the
structural schema (or enable preserveUnknownFields) so the field is
accepted—also apply the same fix for cr-cat-72217.yaml and cr-cron-72217.yaml
where spec.test is present.
| initContainers: | ||
| - name: wait | ||
| image: quay.io/openshifttest/base-alpine@sha256:3126e4eed4a3ebd8bf972b2453fa838200988ee07c01b2251e3ea47e4b1f245c | ||
| command: ["/bin/sh", "-c", "sleep 10"] | ||
| containers: | ||
| - name: hello-pod | ||
| image: quay.io/openshifttest/hello-openshift@sha256:4200f438cf2e9446f6bcff9d67ceea1f69ed07a2f83363b7fb52529f7ddd8a83 | ||
| ports: | ||
| - containerPort: 8080 | ||
| volumeMounts: | ||
| - mountPath: /tmp | ||
| name: tmp | ||
| volumes: |
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
🏁 Script executed:
# First, check if the file exists and examine its full content
find . -name "initContainer.yaml" -type fRepository: openshift/oc
Length of output: 97
🏁 Script executed:
# Also search for the file path mentioned in the review
if [ -f "test/testdata/oc_cli/initContainer.yaml" ]; then
cat -n test/testdata/oc_cli/initContainer.yaml
fiRepository: openshift/oc
Length of output: 811
🏁 Script executed:
# Check context: is there a similar test setup in other testdata files?
find test/testdata -name "*.yaml" -type f | head -10Repository: openshift/oc
Length of output: 556
🏁 Script executed:
# Check if there are other pod fixtures in testdata for comparison
grep -l "kind: Pod" test/testdata/oc_cli/*.yaml 2>/dev/null | head -5Repository: openshift/oc
Length of output: 181
🏁 Script executed:
# Examine one or two similar pod fixtures to see if they have securityContext
for f in $(grep -l "kind: Pod" test/testdata/oc_cli/*.yaml 2>/dev/null | head -2); do
echo "=== $f ==="
head -30 "$f"
doneRepository: openshift/oc
Length of output: 1430
🌐 Web query:
Kubernetes Pod securityContext allowPrivilegeEscalation default value
💡 Result:
allowPrivilegeEscalation is a container-level field (spec.containers[*].securityContext.allowPrivilegeEscalation, not spec.securityContext).
- Default (when unset): effectively
true(privilege escalation is allowed unless you explicitly set it tofalse). (dynatrace.com) - Forced behavior: it is always
trueif the container is privileged or hasCAP_SYS_ADMIN, regardless of what you set. (v1-33.docs.kubernetes.io) - Note: Kubernetes Pod Security Standards (restricted) require this field to be set to
false. (kubernetes.io)
Citations:
- 1: https://www.dynatrace.com/engineering/blog/kubernetes-security-part-3-security-context/?utm_source=openai
- 2: https://v1-33.docs.kubernetes.io/docs/tasks/configure-pod-container/security-context/?utm_source=openai
- 3: https://kubernetes.io/docs/concepts/security/pod-security-standards/?utm_source=openai
Add securityContext to keep this pod valid under restricted policies.
Lines 8-20 omit securityContext for both containers. In clusters enforcing Pod Security "restricted" (or strict SCCs), this causes admission failures because allowPrivilegeEscalation defaults to true. Other test fixtures in this repo (e.g., debugpod_48681.yaml) follow this hardening pattern; add it here for consistency and test portability.
Suggested hardening
spec:
+ securityContext:
+ runAsNonRoot: true
initContainers:
- name: wait
image: quay.io/openshifttest/base-alpine@sha256:3126e4eed4a3ebd8bf972b2453fa838200988ee07c01b2251e3ea47e4b1f245c
command: ["/bin/sh", "-c", "sleep 10"]
+ securityContext:
+ allowPrivilegeEscalation: false
containers:
- name: hello-pod
image: quay.io/openshifttest/hello-openshift@sha256:4200f438cf2e9446f6bcff9d67ceea1f69ed07a2f83363b7fb52529f7ddd8a83
ports:
- containerPort: 8080
volumeMounts:
- mountPath: /tmp
name: tmp
+ securityContext:
+ allowPrivilegeEscalation: false🤖 Prompt for AI Agents
In `@test/testdata/oc_cli/initContainer.yaml` around lines 8 - 20, Add a
securityContext to both the initContainer "wait" and the main container
"hello-pod" to satisfy restricted PodSecurity/SCC rules: set
allowPrivilegeEscalation: false, ensure runAsNonRoot: true (and a non-root
runAsUser if needed), and enable readOnlyRootFilesystem where appropriate;
update the initContainers and containers entries to include these
securityContext fields so the pod is accepted under restricted policies and
matches other fixtures like debugpod_48681.yaml.
Fixing oc client caes for microshift and metal ipv6 profliles