Repository navigation
chore(tekton): drop deprecated fileserver publish task and trigger template - #5414
Conversation
…mplate Both resources still defaulted `publisher-url` to a retired external hostname and neither was listed in its kustomization, so they were never deployed: the only publisher endpoints CI uses today are `https://do2.pingcap.net/publisher` (out-of-cluster delivery from build trigger bindings) and `http://publisher.publisher.svc` (in-cluster tasks and notify triggers). - remove tekton/v1/tasks/delivery/deprecated/publish-fileserver-from-oci-artifact.yaml - remove tekton/v1/triggers/templates/_/deprecated/push-oci-artifact-to-fileserver.yaml - drop the commented-out kustomization entries - fix the refresh-tiup.sh usage example, which pointed at a deleted staging host - refresh the CD worker skill note that still listed the removed task
There was a problem hiding this comment.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary:
This PR removes two deprecated Tekton resources related to publishing to a fileserver from OCI artifacts, which are no longer used and would fail due to updated authentication requirements. It also updates related documentation and scripts to reflect the current publisher URL and removes dead code references. The approach is straightforward—delete unused files, clean up kustomization entries, and fix usage examples. Overall, the changes are clear, well-justified, and maintain repository hygiene.
Critical Issues
- None found. The removal of dead code and updating references is safe and well-validated.
Code Improvements
-
Improve commit message or PR description with explicit note on backward compatibility or migration guidance
While the PR description is thorough, consider adding a brief note or link on what users should do if they relied on the deprecated tasks or triggers before. For example:"If you previously used the deprecated publish-fileserver-from-oci-artifact task or trigger, switch to
pingcap-deliver-binariesand update your pipelines accordingly."This helps future readers/users during migration.
-
In
scripts/flow/rc/refresh-tiup.sh(line 101):
The example update is good, but consider adding a comment or link to documentation about the new publisher URL to help users understand the change. For example:# For production usage, point to the new publisher URL: # https://do2.pingcap.net/publisher ./scripts/flow/rc/refresh-tiup.sh https://do2.pingcap.net/publisher <git-branch-name>
-
In
.agents/skills/tekton-cd-backend-worker/SKILL.md(line 67):
The note about the legacy task removal is helpful; consider adding a date or version tag in a standard format to make it easier to parse programmatically, e.g.:- The legacy fileserver publish task and its trigger template were removed in October 2026 (vX.Y.Z); use `pingcap-deliver-binaries`...
Best Practices
-
Documentation:
The removed YAML files contained inline comments describing their workflow. Since these files are deleted, consider creating a short deprecation or migration note in the repository docs or wiki referencing the removal and preferred alternatives. -
Testing:
The PR description indicates kustomize builds succeed and scripts are verified, which is good. However, it’s unclear if there are any automated tests that validate the removal or that the updatedrefresh-tiup.shscript works correctly with the new URL. Consider adding or updating tests or CI checks to catch regressions in these flows. -
Code Style:
No style issues found in the modified scripts or documentation.
Summary of actionable suggestions:
- Add a migration note or comment for users of the deprecated tasks/triggers.
- Add a brief explanation or link about the new publisher URL in the updated script example.
- Enhance the SKILL.md note with a standard date/version format.
- Consider adding or updating automated tests or CI checks to verify the new publisher URL usage and absence of the deprecated tasks/triggers.
- Add a deprecation/migration note in repo documentation to supplement the removed inline comments from deleted YAML.
These changes will improve usability and maintainability for users transitioning away from these deprecated resources.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wuhuizuo The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Summary
tekton/v1/tasks/delivery/deprecated/publish-fileserver-from-oci-artifact.yamlandtekton/v1/triggers/templates/_/deprecated/push-oci-artifact-to-fileserver.yamlstill defaultpublisher-urlto an external hostname that CI no longer uses, and neither file is listed in its kustomization, so both were dead code in the repo (never rendered by Flux).The publisher endpoints CI actually uses are:
https://do2.pingcap.net/publishertekton/v1/triggers/bindings/gcp-{classic,internal,ng}-build-params.yamlhttp://publisher.publisher.svctekton/v1/tasks/**,tekton/v1/step-actions/**,tekton/v1/triggers/triggers/env-gcp/_/notify/**Re-enabling the deprecated pair as-is would fail: the external hostname now sits behind OIDC/JWT, so the plain
curl/publisher-clicalls in those files would get a redirect or a 401.Changes
scripts/flow/rc/refresh-tiup.shusage example, which pointed at a staging hostname that no longer exists; it now useshttps://do2.pingcap.net/publisher.Validation
kustomize build tekton/v1/tasksandkustomize build tekton/v1/triggers/templatespass..ci/update-tekton-kustomizations.shreports no pending kustomization changes..ci/check-pr-content-policy.shreports no violations.grepfinds no remaining references to the removed resources.Follow-up
None in this repo. The ee-ops side (removing the unused external publisher route) is tracked separately.