-
Notifications
You must be signed in to change notification settings - Fork 231
fix: prevent unintended directory overwrite from PR #3057 #3079
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
fix: prevent unintended directory overwrite from PR #3057 #3079
Conversation
513e01f to
c7cea46
Compare
|
/hold cancel |
|
/kind bug |
|
/assign @pramodbindal |
hack/fetch-releases.sh
Outdated
| # pruner (event-based) should not use tekton- prefix | ||
| # to avoid conflicts with tekton-pruner (job-based) directory | ||
| if [[ $comp == "pruner" ]]; then | ||
| dir="pruner" |
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.
dir you already passing as pruner so i think we don't need this condition
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.
🥹 Thankyou for pointing out. Removed the redundant lines.
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.
what is the purpose of renaming this file ?
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.
This has to match the filepath in the release_test.go. (This is how it was in previous versions.) https://github.com/tektoncd/operator/blob/release-v0.77.x/pkg/reconciler/common/testdata/kodata/pruner/0.1.0/release-v0.1.0.yaml
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.
what is the purpose of renaming this file ?
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.
This is added newly to the tests since this operator version consumes the latest version(0.3.3) of pruner. Ideally every component should have this updated for every new version adopted in Operator
Signed-off-by: Anitha Natarajan <[email protected]>
d784545 to
e5621d6
Compare
|
/lgtm |
|
/approved |
|
/approve |
|
@anithapriyanatarajan Here pruner yaml manifest dir should be |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mbpavan, pramodbindal The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/cherry-pick release-v0.78.x |
|
✅ Cherry-pick to A new pull request has been created to cherry-pick this change to Please review and merge the cherry-pick PR. |
Changes
This pull request is to correct an unintended directory overwrite issue introduced by PR #3057 resulting in the config files required for old job based pruner being completely removed from the Operator manifests.
To correct the naming conflict between two types of pruner components (event-based and job-based) directory references in both the shell script and Go code are updated with this PR. The main change ensures that the event-based pruner uses the
prunerdirectory instead oftekton-pruner, preventing potential clashes with the job-based pruner.hack/fetch-releases.sh, updated the logic so that the event-based pruner component uses theprunerdirectory instead oftekton-pruner, with comments explaining the reason for this change.pkg/reconciler/common/releases.go, modified theComponentDirfunction to return theprunerdirectory for event-based pruner components, with explanatory comments to clarify the distinction.Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make test lintbefore submitting a PRSee the contribution guide for more details.
Release Notes