ESO-276: Bump golang packages of the operator#101
ESO-276: Bump golang packages of the operator#101PillaiManish wants to merge 6 commits intoopenshift:mainfrom
Conversation
|
@PillaiManish: This pull request references ESO-276 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 story to target either version "4.22." or "openshift-4.22.", but it targets "eso-1.1" 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. |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
do-not-merge/work-in-progress
WalkthroughRefactors build tooling and test modules, updates Kubernetes and controller-runtime dependencies, adds separate test and tools Go modules, tweaks Makefile targets and variables, updates Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
PillaiManish
force-pushed
the
ESO-276-segregate-go-packages-with-go-workspace
branch
2 times, most recently
from
7187f1d to
e0fc989
Compare
|
@PillaiManish: This pull request references ESO-276 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 story to target either version "4.22." or "openshift-4.22.", but it targets "eso-1.1" 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. |
|
@PillaiManish: This pull request references ESO-276 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 story to target either version "4.22." or "openshift-4.22.", but it targets "eso-1.1" 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. |
|
@PillaiManish: This pull request references ESO-276 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 story to target either version "4.22." or "openshift-4.22.", but it targets "eso-1.1" 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. |
PillaiManish
force-pushed
the
ESO-276-segregate-go-packages-with-go-workspace
branch
from
e0fc989 to
c0215d8
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: PillaiManish 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 |
PillaiManish
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
@PillaiManish: This pull request references ESO-276 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 story to target either version "4.22." or "openshift-4.22.", but it targets "eso-1.1" 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. |
openshift-merge-robot
added
the
needs-rebase
Create Go workspace structure with separate modules: - go.work: Links main, tools/, and test/e2e/ modules - tools/go.mod: Development tools (controller-gen, golangci-lint, etc.) - test/e2e/go.mod: E2E test dependencies - go.mod: Production code only (tool deps removed) - Updated vendor/ via 'go work vendor'
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@Makefile`:
- Around line 148-149: The test-unit Makefile target currently uses a bad regex
in the go test pipeline (inside the GOFLAGS="" go test ... command) — replace
the character-class pattern 'test/[e2e|apis|utils]' with a grouped alternation
pattern like 'test/(e2e|apis|utils)' so grep -vE excludes only those
directories; update the command in the test-unit target accordingly to use the
corrected regex.
In `@test/utils/external_secrets.go`:
- Around line 101-103: The current use of strings.Replace on wd (the working
directory variable) can remove "/test" anywhere; change the logic to only strip
these suffixes from the end of wd by checking and removing "/test/e2e" first
then "/test" (e.g., use strings.HasSuffix + strings.TrimSuffix or
strings.TrimSuffix directly in that order) so you only trim the directory suffix
and avoid accidental mid-path removal; update the code around the wd assignment
that currently calls strings.Replace to perform suffix-only trimming.
| // Handle both old path (test/e2e) and new path (test) structures | ||
| wd = strings.Replace(wd, "/test/e2e", "", -1) | ||
| wd = strings.Replace(wd, "/test", "", -1) |
There was a problem hiding this comment.
Fix path trimming to avoid accidental mid-path removal.
Line 102/103 uses strings.Replace(..., -1), which can strip /test anywhere in the path (e.g., /home/testuser/...), breaking project directory resolution.
🔧 Proposed fix (suffix-only trimming)
- wd = strings.Replace(wd, "/test/e2e", "", -1)
- wd = strings.Replace(wd, "/test", "", -1)
+ wd = strings.TrimSuffix(wd, "/test/e2e")
+ wd = strings.TrimSuffix(wd, "/test")📝 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.
| // Handle both old path (test/e2e) and new path (test) structures | |
| wd = strings.Replace(wd, "/test/e2e", "", -1) | |
| wd = strings.Replace(wd, "/test", "", -1) | |
| // Handle both old path (test/e2e) and new path (test) structures | |
| wd = strings.TrimSuffix(wd, "/test/e2e") | |
| wd = strings.TrimSuffix(wd, "/test") |
🤖 Prompt for AI Agents
In `@test/utils/external_secrets.go` around lines 101 - 103, The current use of
strings.Replace on wd (the working directory variable) can remove "/test"
anywhere; change the logic to only strip these suffixes from the end of wd by
checking and removing "/test/e2e" first then "/test" (e.g., use
strings.HasSuffix + strings.TrimSuffix or strings.TrimSuffix directly in that
order) so you only trim the directory suffix and avoid accidental mid-path
removal; update the code around the wd assignment that currently calls
strings.Replace to perform suffix-only trimming.
Makefile updates for Go workspace: - Add TOOLS_DIR, PROJECT_ROOT, CONTROLLER_GEN_RUN, SETUP_ENVTEST_RUN vars - Update manifests/generate targets to use explicit paths - Add GOFLAGS="" prefix to avoid workspace conflicts - Update go-install-tool to build from tools module - Add update-vendor and update-dep helper targets - Add verify-deps target - Update test-e2e to run from test/e2e directory Also updated .gitignore to track go.work files.
Update Kubernetes and related dependencies for OpenShift 4.21: - k8s.io/* packages: v0.34.1 → v0.34.3 - k8s.io/kubernetes: v1.34.1 → v1.34.3 - sigs.k8s.io/controller-runtime: v0.22.3 → v0.22.5 - github.com/golangci/golangci-lint/v2: v2.7.2 → v2.8.0 - Various transitive dependency updates
This change restructures the test module to support broader test requirements: - Move go.mod from test/e2e to test directory - Update module name to github.com/openshift/external-secrets-operator/test - Update go.work to reference ./test instead of ./test/e2e - Update Makefile targets (test-e2e, update-vendor, verify-deps, update-dep) - Update GetProjectDir() in test/utils to handle new path structure - Clean up root go.mod by moving test-specific dependencies to test module - Add operator-sdk/ to .gitignore This allows the test module to be used for all test-related code (e2e tests, API tests, utils) and supports adding new tools like API validation. Reference: openshift/cert-manager-operator PR #369
PillaiManish
force-pushed
the
ESO-276-segregate-go-packages-with-go-workspace
branch
from
d68ffa2 to
7a6168a
Compare
openshift-merge-robot
removed
the
needs-rebase
- Add GOWORK=off and GOFLAGS="-mod=readonly" to fmt, vet, test-unit, and test-e2e targets - This prevents 'inconsistent vendoring' errors when vendor was created with go work vendor - The vendor directory contains dependencies from all workspace modules, but Go checks consistency against only the current go.mod - Disabling workspace mode and using readonly mode skips the vendor consistency check - Also fixed regex pattern in test-unit grep from [e2e|apis|utils] to (e2e|apis|utils)
The test directory is a separate Go module, so we need to cd into it and run 'go test ./e2e' instead of running 'go test ./test/e2e' from the main module context.
|
@PillaiManish: all tests passed! 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. |
3 participants
Bumps golang packages and segregates Go packages into separate modules based on usage. This PR implements Go Workspaces for multi-module dependency management.
Motivation
To reduce CVE reports on development tool and e2e dependencies that are not part of the production binary
Changes