MULTIARCH-5550: add vault test for ppc64le#134
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Vault-focused e2e test and test fixtures plus a new Makefile target to run the Vault e2e suite: deploys Vault, initializes/unseals/logs in, configures KV v2, writes a secret, creates SecretStore/ExternalSecret/ExternalSecretsConfig and NetworkPolicy, verifies retrieved secret, and performs cleanup. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
test/e2e/e2e_test.go (2)
726-727: Remove unused constants.
clusterSecretStoreFileandexternalSecretFileare declared but never used. The actual file paths are defined locally in the test function (lines 790-792).Context("Vault Secret Manager", Label("Platform:Vault"), func() { const ( - clusterSecretStoreFile = "testdata/vault/secret_store.yaml" - externalSecretFile = "testdata/vault/external_secret.yaml" vaultSecretName = "foo" vaultSecretKey = "my-value" vaultSecretValue = "bar" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/e2e_test.go` around lines 726 - 727, Remove the unused constants clusterSecretStoreFile and externalSecretFile from the top-level declarations in the test file; they are never referenced and duplicate the local paths that are used inside the test function (where the actual file paths are defined around lines ~790-792). Locate and delete the constant definitions named clusterSecretStoreFile and externalSecretFile so only the locally scoped variables remain.
1031-1037: Shell command construction may break with special characters.The
fmt.Sprintfconstructs a shell command with direct string interpolation. Ifsecretname,key, orvaluecontain shell metacharacters (quotes, backticks,$), the command could fail or behave unexpectedly.For test code with controlled inputs this is low risk, but consider using separate arguments to
vault kv putto avoid shell interpretation issues.Alternative: pass key-value without shell quoting
cmd := exec.Command( - "oc", "exec", "-n", vaultNamespace, podName, "--", "sh", "-c", - fmt.Sprintf( - "vault kv put secret/%s %s=\"%s\"", - secretname, key, value, - ), + "oc", "exec", "-n", vaultNamespace, podName, "--", + "vault", "kv", "put", fmt.Sprintf("secret/%s", secretname), + fmt.Sprintf("%s=%s", key, value), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/e2e_test.go` around lines 1031 - 1037, The test builds a shell command via fmt.Sprintf and sh -c which allows shell metacharacters in secretname/key/value to break behavior; update the exec.Command invocation (the cmd := exec.Command(...) that currently uses "sh", "-c" and fmt.Sprintf("vault kv put secret/%s %s=\"%s\"")) to call vault kv put with separate arguments instead of a shell string—e.g., remove "sh", "-c" and pass "vault","kv","put", fmt.Sprintf("secret/%s", secretname), fmt.Sprintf("%s=%s", key, value) (or construct the key=value arg safely) so that exec.Command handles argument quoting and no shell interpolation occurs.Makefile (1)
219-230: Inconsistencies with existingtest-e2etarget may cause issues.The new
e2e-test-vaulttarget differs fromtest-e2ein several ways that could cause problems:
- Missing
GOFLAGS=override — line 182 sets this fortest-e2eto avoid conflicts withgo.work, bute2e-test-vaultis not included- Missing
@prefix to suppress command echo- Missing
-ginkgo.traceflag- Different working directory approach (
./test/e2evs-C $(PROJECT_ROOT)/test ./e2e)Suggested fix for consistency
- -.PHONY: e2e-test-vault # Run the e2e tests against a Kind k8s instance that is spun up. + +# Targets that need Go workspace mode (CI sets GOFLAGS=-mod=vendor which conflicts with go.work) +fmt vet test test-unit test-e2e e2e-test-vault run update-vendor update-dep: GOFLAGS= + +.PHONY: e2e-test-vault e2e-test-vault: - go test \ + `@go` test -C $(PROJECT_ROOT)/test \ -timeout $(E2E_TIMEOUT) \ -count 1 \ -v \ -p 1 \ -tags e2e \ - ./test/e2e \ + ./e2e \ -ginkgo.v \ + -ginkgo.trace \ -ginkgo.show-node-events \ -ginkgo.label-filter="$(E2E_GINKGO_LABEL_FILTER)"Note: Update line 182 to include
e2e-test-vaultin the GOFLAGS override list.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 219 - 230, The new e2e-test-vault Make target is inconsistent with test-e2e; update e2e-test-vault to match: ensure it's covered by the GOFLAGS override (add e2e-test-vault to the existing GOFLAGS= override that prevents go.work conflicts), prefix the go test invocation with @ to suppress echo, add the -ginkgo.trace flag to the ginkgo flags, and run the tests from the same directory style as test-e2e (use -C $(PROJECT_ROOT)/test ./e2e rather than ./test/e2e) while keeping the same flags (-timeout $(E2E_TIMEOUT) -count 1 -v -p 1 -tags e2e -ginkgo.v -ginkgo.show-node-events -ginkgo.label-filter="$(E2E_GINKGO_LABEL_FILTER)").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/e2e_test.go`:
- Around line 762-785: The AfterEach cleanup currently removes Vault
namespace/networkpolicy and also deletes the global ExternalSecretsConfig
"cluster"; change this block to run in AfterAll (replace AfterEach with
AfterAll) so cleanup runs once, and remove the deletion of the shared
ExternalSecretsConfig "cluster" (do not call safeDelete for
ExternalSecretsConfig "cluster"); if you need to revert Vault-specific config
changes, instead restore the ExternalSecretsConfig to its prior state (or delete
only Vault-scoped resources) using the same safeDelete helper and referencing
vaultNamespace, networkpolicy name "allow-to-vault-test", and the
ExternalSecretsConfig resource only when scoped to Vault-specific changes.
- Around line 1044-1069: In createVaultTokenSecret, don’t assume any Get() error
means “not found”: use the Kubernetes API error helper
(apierrors.IsNotFound(err)) to distinguish a missing secret from other errors;
keep the existing update path when err == nil, call Create only when
apierrors.IsNotFound(err), and for any other non-nil error return it immediately
(adjust imports to include k8s.io/apimachinery/pkg/api/errors as apierrors).
In `@test/e2e/testdata/vault/externalsecretsconfig.yaml`:
- Around line 10-14: The network policy for ExternalSecretsCoreController is
overly permissive because the egress entry `- {}` (in the policy named
`allow-external-secrets-egress`) allows all outbound traffic; update the
`networkPolicies` for componentName `ExternalSecretsCoreController` to either
remove this empty egress rule if redundant with `vault-networkpolicy.yaml` or
replace it with explicit egress rules (e.g., allow TCP/UDP port 53 for DNS and
TCP port 8200 to the Vault namespace) so only required destinations/ports are
permitted.
---
Nitpick comments:
In `@Makefile`:
- Around line 219-230: The new e2e-test-vault Make target is inconsistent with
test-e2e; update e2e-test-vault to match: ensure it's covered by the GOFLAGS
override (add e2e-test-vault to the existing GOFLAGS= override that prevents
go.work conflicts), prefix the go test invocation with @ to suppress echo, add
the -ginkgo.trace flag to the ginkgo flags, and run the tests from the same
directory style as test-e2e (use -C $(PROJECT_ROOT)/test ./e2e rather than
./test/e2e) while keeping the same flags (-timeout $(E2E_TIMEOUT) -count 1 -v -p
1 -tags e2e -ginkgo.v -ginkgo.show-node-events
-ginkgo.label-filter="$(E2E_GINKGO_LABEL_FILTER)").
In `@test/e2e/e2e_test.go`:
- Around line 726-727: Remove the unused constants clusterSecretStoreFile and
externalSecretFile from the top-level declarations in the test file; they are
never referenced and duplicate the local paths that are used inside the test
function (where the actual file paths are defined around lines ~790-792). Locate
and delete the constant definitions named clusterSecretStoreFile and
externalSecretFile so only the locally scoped variables remain.
- Around line 1031-1037: The test builds a shell command via fmt.Sprintf and sh
-c which allows shell metacharacters in secretname/key/value to break behavior;
update the exec.Command invocation (the cmd := exec.Command(...) that currently
uses "sh", "-c" and fmt.Sprintf("vault kv put secret/%s %s=\"%s\"")) to call
vault kv put with separate arguments instead of a shell string—e.g., remove
"sh", "-c" and pass "vault","kv","put", fmt.Sprintf("secret/%s", secretname),
fmt.Sprintf("%s=%s", key, value) (or construct the key=value arg safely) so that
exec.Command handles argument quoting and no shell interpolation occurs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf609882-7f2b-4bc5-a693-4dddaa3125dc
📒 Files selected for processing (7)
Makefiletest/e2e/e2e_test.gotest/e2e/testdata/vault/cluster_secret_store.yamltest/e2e/testdata/vault/external_secret.yamltest/e2e/testdata/vault/externalsecretsconfig.yamltest/e2e/testdata/vault/vault-networkpolicy.yamltest/e2e/testdata/vault/vault.yaml
There was a problem hiding this comment.
🧹 Nitpick comments (3)
test/e2e/e2e_test.go (3)
725-731: Remove unused constants.
clusterSecretStoreFileandexternalSecretFileare declared but never used. The test instead defines local variablesvaultSecretStoreFileandvaultExternalSecretFileat lines 784-785.Suggested fix
Context("Vault Secret Manager", Label("Platform:Vault"), func() { const ( - clusterSecretStoreFile = "testdata/vault/secret_store.yaml" - externalSecretFile = "testdata/vault/external_secret.yaml" - vaultSecretName = "foo" - vaultSecretKey = "my-value" - vaultSecretValue = "bar" + vaultSecretName = "foo" + vaultSecretKey = "my-value" + vaultSecretValue = "bar" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/e2e_test.go` around lines 725 - 731, Remove the unused constants clusterSecretStoreFile and externalSecretFile from the constants block (they are shadowed by local variables vaultSecretStoreFile and vaultExternalSecretFile later in the test), leaving only the actually used constants vaultSecretName, vaultSecretKey, and vaultSecretValue; update the constants declaration near the top of the test in e2e_test.go so there are no unused variables left.
874-874: Remove unusedloaderparameter.The
loaderparameter is passed toapplyVaultbut never used within the function.Suggested fix
-func applyVault(ctx context.Context, loader utils.DynamicResourceLoader) error { +func applyVault(ctx context.Context) error {And update the call site at line 735:
- Expect(applyVault(ctx, loader)).To(Succeed()) + Expect(applyVault(ctx)).To(Succeed())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/e2e_test.go` at line 874, The applyVault function signature includes an unused parameter loader; remove the unused parameter from the applyVault declaration (change func applyVault(ctx context.Context, loader utils.DynamicResourceLoader) to func applyVault(ctx context.Context)) and update all call sites that pass a loader to call applyVault with only the ctx argument (i.e., remove the extra argument where applyVault is invoked) so the function and its callers remain consistent.
1004-1014: Consider more granular error handling inenableKVEngine.The
|| trueat line 1007 masks all errors, including authentication failures or Vault unavailability. While this is intentional for idempotent KV engine enabling, consider checking only for the "already enabled" error.Alternative approach
cmd := exec.Command( "oc", "exec", "-n", vaultNamespace, podName, "--", "sh", "-c", fmt.Sprintf( - "vault status && vault login %s && vault secrets enable -path=secret kv-v2 || true", + "vault status && vault login %s && (vault secrets enable -path=secret kv-v2 2>&1 || echo 'KV engine may already be enabled')", token, ), )Alternatively, check the exit code explicitly and only ignore the "path is already in use" error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/e2e_test.go` around lines 1004 - 1014, The current command uses "|| true" which masks all errors; in the enableKVEngine logic replace the blanket ignore with targeted handling: run the same exec.Command via utils.Run (the existing cmd, podName, vaultNamespace, token) but if it returns an error inspect the output/error string or exit code and only suppress the error when it indicates the KV engine is already enabled (e.g., message like "path is already in use" or equivalent); for any other error (authentication failure, vault unavailable, etc.) propagate/return the error so failures are not silently ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/e2e/e2e_test.go`:
- Around line 725-731: Remove the unused constants clusterSecretStoreFile and
externalSecretFile from the constants block (they are shadowed by local
variables vaultSecretStoreFile and vaultExternalSecretFile later in the test),
leaving only the actually used constants vaultSecretName, vaultSecretKey, and
vaultSecretValue; update the constants declaration near the top of the test in
e2e_test.go so there are no unused variables left.
- Line 874: The applyVault function signature includes an unused parameter
loader; remove the unused parameter from the applyVault declaration (change func
applyVault(ctx context.Context, loader utils.DynamicResourceLoader) to func
applyVault(ctx context.Context)) and update all call sites that pass a loader to
call applyVault with only the ctx argument (i.e., remove the extra argument
where applyVault is invoked) so the function and its callers remain consistent.
- Around line 1004-1014: The current command uses "|| true" which masks all
errors; in the enableKVEngine logic replace the blanket ignore with targeted
handling: run the same exec.Command via utils.Run (the existing cmd, podName,
vaultNamespace, token) but if it returns an error inspect the output/error
string or exit code and only suppress the error when it indicates the KV engine
is already enabled (e.g., message like "path is already in use" or equivalent);
for any other error (authentication failure, vault unavailable, etc.)
propagate/return the error so failures are not silently ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92efc958-9b30-428c-b949-6e886bc8d253
📒 Files selected for processing (1)
test/e2e/e2e_test.go
|
/retitle MULTIARCH-5550: add vault test for ppc64le |
openshift-ci
Bot
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@raja-0940: This pull request references MULTIARCH-5550 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target either version "4.22." or "openshift-4.22.", but it targets "openshift-4.21" instead. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
test/e2e/testdata/vault/vault.yaml (1)
53-84: Consider adding resource requests/limits.The container lacks resource requests and limits. While acceptable for e2e tests, defining them can prevent resource contention and improve scheduling predictability in shared test clusters.
💡 Optional: Add resource constraints
ports: - containerPort: 8200 securityContext: allowPrivilegeEscalation: false capabilities: drop: - ALL + resources: + requests: + cpu: 100m + memory: 256Mi + limits: + cpu: 500m + memory: 512Mi volumeMounts:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/testdata/vault/vault.yaml` around lines 53 - 84, The vault container spec (container name "vault") is missing resources; add a resources block under the container to set requests and limits (e.g., resources.requests.cpu, resources.requests.memory and resources.limits.cpu, resources.limits.memory) so the "vault" container has both minimal guaranteed resources and caps; update the same container entry that contains command/args/readinessProbe/volumeMounts to include these fields with appropriate values for the e2e environment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 219-230: The Makefile target e2e-test-vault is missing from the
target-specific GOFLAGS reset list, causing exported GOFLAGS (e.g. -mod=vendor)
to be applied and break workspace mode; update the GOFLAGS override list (the
target-specific GOFLAGS= reset block) to include e2e-test-vault so that the
e2e-test-vault target uses a clean GOFLAGS environment when invoking go test.
- Line 230: Remove the extraneous double quotes around E2E_GINKGO_LABEL_FILTER
so the ginkgo.label-filter uses the already-quoted variable (match the pattern
used by test-e2e for E2E_GINKGO_LABEL_FILTER), and update the GOFLAGS reset list
to include the e2e-test-vault target so it does not inherit CI's -mod=vendor
(add e2e-test-vault to the list where GOFLAGS is cleared/reset).
In `@test/e2e/testdata/vault/vault.yaml`:
- Around line 1-5: The YAML file defining a Namespace (kind: Namespace,
metadata.name: vault-test) uses incorrect line endings; convert the file
test/e2e/testdata/vault/vault.yaml to use Unix-style LF line endings (\n)
throughout (ensure no CRLF sequences), then re-save/commit the file so CI and
parsers read the manifest correctly.
---
Nitpick comments:
In `@test/e2e/testdata/vault/vault.yaml`:
- Around line 53-84: The vault container spec (container name "vault") is
missing resources; add a resources block under the container to set requests and
limits (e.g., resources.requests.cpu, resources.requests.memory and
resources.limits.cpu, resources.limits.memory) so the "vault" container has both
minimal guaranteed resources and caps; update the same container entry that
contains command/args/readinessProbe/volumeMounts to include these fields with
appropriate values for the e2e environment.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12c742d1-a59f-434a-b8d0-27caf760dca9
📒 Files selected for processing (7)
Makefiletest/e2e/e2e_test.gotest/e2e/testdata/vault/cluster_secret_store.yamltest/e2e/testdata/vault/external_secret.yamltest/e2e/testdata/vault/externalsecretsconfig.yamltest/e2e/testdata/vault/vault-networkpolicy.yamltest/e2e/testdata/vault/vault.yaml
✅ Files skipped from review due to trivial changes (3)
- test/e2e/testdata/vault/external_secret.yaml
- test/e2e/testdata/vault/externalsecretsconfig.yaml
- test/e2e/testdata/vault/vault-networkpolicy.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- test/e2e/testdata/vault/cluster_secret_store.yaml
- test/e2e/e2e_test.go
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/e2e/testdata/vault/vault.yaml (1)
1-5:⚠️ Potential issue | 🟡 MinorFix newline character format.
Static analysis indicates the file uses incorrect line endings (CRLF). Convert to Unix-style line endings (
\n) for CI compatibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/testdata/vault/vault.yaml` around lines 1 - 5, The YAML file defining the Namespace (look for the block with "apiVersion: v1", "kind: Namespace", and "name: vault-test") uses CRLF line endings and must be converted to Unix-style LF; open that vault.yaml, normalize line endings to "\n" (e.g., use your editor's EOL conversion, run dos2unix, or git's core.autocrlf settings), verify there are no trailing CR (\r) characters, and recommit the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/e2e/testdata/vault/vault.yaml`:
- Around line 1-5: The YAML file defining the Namespace (look for the block with
"apiVersion: v1", "kind: Namespace", and "name: vault-test") uses CRLF line
endings and must be converted to Unix-style LF; open that vault.yaml, normalize
line endings to "\n" (e.g., use your editor's EOL conversion, run dos2unix, or
git's core.autocrlf settings), verify there are no trailing CR (\r) characters,
and recommit the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58ce074d-3ad7-4f54-bfdf-f62ae51dcb00
📒 Files selected for processing (4)
Makefiletest/e2e/e2e_test.gotest/e2e/testdata/vault/externalsecretsconfig.yamltest/e2e/testdata/vault/vault.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/testdata/vault/externalsecretsconfig.yaml
|
/retitle MULTIARCH-5550: add vault test for ppc64le |
|
/test verify |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
test/e2e/testdata/vault/vault.yaml (1)
1-115:⚠️ Potential issue | 🟡 MinorRe-save this manifest with LF line endings.
YAMLlint is still reporting CRLF here, so CI will keep failing until the file is saved with Unix newlines.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/testdata/vault/vault.yaml` around lines 1 - 115, The manifest uses CRLF endings; re-save vault.yaml with Unix LF line endings so YAML lint passes (affects resources like the Namespace named "vault-test", Deployment/ServiceAccount/Deployment named "vault", and ConfigMap "vault-config"); open the file in your editor or run a tool (e.g., convert line endings or dos2unix) to normalize to LF, commit the updated file, and ensure your local git autocrlf setting won't reintroduce CRLF.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/e2e_test.go`:
- Around line 918-924: The test currently prints the raw Vault command outputs
(initOut from utils.Run(initCmd) and similarly login output) via
By(fmt.Sprintf(...)), leaking unseal keys and tokens; change the test to NOT
print raw command output to logs/artifacts—keep capturing initOut/loginOut for
error handling but replace the By(fmt.Sprintf(...)) calls with sanitized status
messages like "Vault operator init completed" and "Vault login completed", and
if you must log details, redact sensitive fields from initOut/loginOut before
logging or only log non-sensitive status lines; ensure functions/variables
initOut, initCmd, and the By(...) calls are updated accordingly.
- Around line 762-767: The test suite currently spawns oc subprocesses with
exec.Command which can hang; change calls in functions that accept a context
(setupVault, enableKVEngine, createVaultTestSecret) to use
exec.CommandContext(ctx, ...) so they respect the caller's timeout, and update
all cleanup invocations to create a short deadline context (context.WithTimeout)
and call a modified safeDelete(ctx, cmd) that uses exec.CommandContext
internally; specifically update safeDelete to accept a context.Context and run
the oc delete/apply/exec commands via exec.CommandContext, and replace the
listed direct exec.Command usages (around the safeDelete calls and the lines for
setupVault/enableKVEngine/createVaultTestSecret) to use the context-aware
variants so subprocesses are bounded by deadlines.
---
Duplicate comments:
In `@test/e2e/testdata/vault/vault.yaml`:
- Around line 1-115: The manifest uses CRLF endings; re-save vault.yaml with
Unix LF line endings so YAML lint passes (affects resources like the Namespace
named "vault-test", Deployment/ServiceAccount/Deployment named "vault", and
ConfigMap "vault-config"); open the file in your editor or run a tool (e.g.,
convert line endings or dos2unix) to normalize to LF, commit the updated file,
and ensure your local git autocrlf setting won't reintroduce CRLF.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16992a33-62aa-4ee9-93a4-e0710accdc67
📒 Files selected for processing (3)
Makefiletest/e2e/e2e_test.gotest/e2e/testdata/vault/vault.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: prb112, raja-0940 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 |
|
@swghosh Could you please review this PR? |
|
/retest-required |
|
Hi @swghosh - our tests are passing on IBM Power. We'd appreciate a merge so we can start testing it. Thank you, Paul |
hey @prb112 cc: @bharath-b-rh as he is leading the ESO efforts now. |
|
Thank you @mytreya-rh Hi @bharath-b-rh happy to take feedback, many thanks in advance, Paul |
|
Thank you @prb112 ! I will take a look tomorrow or early next week. |
bharath-b-rh
left a comment
bharath-b-rh
left a comment
There was a problem hiding this comment.
Please update the PR description with more details to help with the reviews.
|
@raja-0940: This pull request references MULTIARCH-5550 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target either version "5.0." or "openshift-5.0.", but it targets "openshift-4.21" instead. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
New changes are detected. LGTM label has been removed. |
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
raja-0940
force-pushed
the
eso-e2e-clean
branch
2 times, most recently
from
5297740 to
977b69c
Compare
Signed-off-by: Rajakumar Battula <rbattula@redhat.com>
|
Hi @bharath-b-rh, I have resolved all the PR review comments. Could you please review them again? Thanks |
|
Hi @raja-0940 , Thank you for addressing the comments! I will take a look at the changes on Thursday. |
|
/retest-required |
|
@raja-0940: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
Hey @bharath-b-rh looking at the retest, I think this is saying the go.mod is missing a dependency? I think we've got it all covered already, any advice. Thank you, Paul p.s. Also happy to make changes based on review comments. cc: @raja-0940 |
6 participants
Summary
Adds e2e test suite for vault with IBM power.
Description
Platform:Vault - ESO CR-based flow:
ExternalSecretsConfigBeforeAllClusterSecretStoreExternalSecretAfterAllTesting