Skip to content
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

feat(apps/prod/tekton/configs): add task and triggers for auto sync OWNERS for hotfix branches #737

Conversation

wuhuizuo
Copy link
Collaborator

@wuhuizuo wuhuizuo commented Oct 24, 2023

Automatically create pull requests when hot fix branch is created:

  1. Setup the Tekton triggers and tasks to watch the repositories on branch creating events.
  2. Call the Deno script to create pull request to the target branch if the matched branch is created.

…indings

- tag creating event
- branch creating event
- branch pushing event

Signed-off-by: wuhuizuo <wuhuizuo@126.com>
@ti-chi-bot ti-chi-bot bot requested review from jayl1e and purelind October 24, 2023 09:54
@ti-chi-bot ti-chi-bot bot added area/apps env/prod will deploy on the main product cluster labels Oct 24, 2023
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Oct 24, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.
This pull request adds a task and triggers for automatically syncing OWNERS for hotfix branches. It also modifies the git-ref value in the github-branch-create.yaml, github-branch-push.yaml, and github-tag-create.yaml files to remove the prefix refs/heads/ or refs/tags/.

One potential problem with this pull request is that it does not provide enough context about how the OWNERS file will be synced. It would be helpful to have more information on how this task and trigger will work to ensure that it is properly implemented.

Another potential problem is that the modified git-ref values in the github-branch-create.yaml, github-branch-push.yaml, and github-tag-create.yaml files are now different from each other. It would be best to keep these values consistent across all three files to avoid confusion and potential errors.

To fix these issues, the pull request author should provide more information about how the OWNERS file will be synced. The author should also modify the git-ref values to be consistent across all three files.

@ti-chi-bot ti-chi-bot bot added the size/S label Oct 24, 2023
@wuhuizuo
Copy link
Collaborator Author

/review default

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Oct 24, 2023

@wuhuizuo:

I have already done a preliminary review for you, and I hope to help you do a better job.
This is a Pull Request to add a new feature that automatically creates pull requests when a hotfix branch is created. The changes contain updates to Tekton triggers and tasks to watch repositories on branch creating events and a call to a Deno script to create pull requests to the target branch if the matched branch is created.

The changes include modifications to the github-branch-create.yaml, github-branch-push.yaml, and github-tag-create.yaml files. The diff shows the changes made in the git-ref parameter of each file. The changes replace the full refs/heads/main or refs/heads/feature with main or feature. Similarly for refs/tags/vX.Y.Z it replaces it with vX.Y.Z

Regarding potential problems, it is not clear how the Deno script works or how it creates pull requests. There could be issues with the implementation of the script that would cause an error. Additionally, there could be problems with the Tekton triggers and tasks that are added, particularly if they interfere with existing triggers and tasks.

As for fixing suggestions, more information on the Deno script would be helpful, along with clear documentation on how to run and test it. It would also be useful to add comments to the Tekton trigger and task code to clarify their purpose and how they work.

In response to this:

/review default

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 kubernetes/test-infra repository.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Oct 24, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.
This pull request adds functionality to automatically create pull requests when a hotfix branch is created. The changes include setting up Tekton triggers and tasks to watch for branch creation events and calling a Deno script to create a pull request to the target branch if the branch matches.

One potential problem with this pull request is that the Deno script could create a pull request for a branch that isn't a hotfix branch, leading to unnecessary pull requests. To avoid this, a check could be added to ensure that only hotfix branches trigger the creation of pull requests.

Another potential issue is that the pull request description doesn't include any information about how to configure and use this new functionality. Adding documentation on how to set up and use the new feature would be helpful for users.

Overall, this pull request looks good, and the changes will likely be useful. Adding a check to ensure only hotfix branches trigger the creation of pull requests and including documentation on how to use the feature would be helpful.

…WNERS for hotfix branches

Automatically create pull requests when hot fix branch is created.

Signed-off-by: wuhuizuo <wuhuizuo@126.com>
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Oct 24, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.

Review of Pull Request titled "feat(apps/prod/tekton/configs): add task and triggers for auto sync OWNERS for hotfix branches"

Summary

The pull request adds functionality to create pull requests automatically when a hotfix branch is created. It sets up Tekton triggers and tasks to monitor branch creation events and calls a Deno script to create the pull request. The pull request also updates the Kustomization YAML file and modifies the bindings and templates for Tekton triggers.

Potential Problems

  • The Deno script is called remotely, which could potentially introduce security vulnerabilities if the script is changed maliciously or if it is hosted on an untrusted server.
  • The Deno script is run with the --allow-all flag, which grants the script full permissions on the system. This could be a security risk and should be avoided, if possible.
  • The create-pr-to-sync-owners.yaml file is missing documentation about how to use it, making it difficult for other developers to understand how to use it.

Fix Suggestions

  • Consider running the Deno script locally instead of calling it remotely to eliminate the security risk.
  • Instead of using the --allow-all flag, use more specific flags to grant only the necessary permissions to the Deno script.
  • Add documentation to the create-pr-to-sync-owners.yaml file to explain how to use it, including details about the expected input parameters, how to modify it for different use cases, and any potential issues that may arise.

Overall, the pull request looks good, but the potential security concerns and lack of documentation should be addressed before merging it into the main branch.

@ti-chi-bot ti-chi-bot bot added size/L and removed size/S labels Oct 24, 2023
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Oct 25, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.
Summary:

This pull request aims to create pull requests automatically when hotfix branches are created. The changes include setting up Tekton triggers and tasks to watch repositories on branch creation events, and calling a Deno script to create a pull request to the target branch when a matched branch is created.

Potential Problems:

  • The Deno script used in the task is fetched from an external URL. It is possible that this URL could be changed or removed in the future, causing the task to fail. It is recommended to save a copy of the script in the repository and use it from there instead.
  • The Deno script uses a private token to authenticate with GitHub. It is recommended to use a secret to store this token and mount it as a volume to the task, rather than embedding it directly in the script.
  • There is no error handling in the Deno script. If an error occurs, the task will fail silently, and it may not be immediately clear what went wrong. It is recommended to add error handling and logging to the script to aid in debugging.

Fixing Suggestions:

  • Save a copy of the Deno script in the repository and use it from there instead of fetching it from an external URL.
  • Use a secret to store the private token used to authenticate with GitHub, and mount it as a volume to the task.
  • Add error handling and logging to the Deno script to aid in debugging.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Oct 25, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.
This pull request adds a new Tekton task for syncing the OWNERS file to GitHub repository branches, and sets up the Tekton triggers to call the task when a new hotfix branch is created. The changes look good, but there is room for improvement in the following areas:

  1. The clone-community-repo step in the create-pr-to-sync-owners task clones the community repository on every run. It would be better to clone it only when the repository is not already present.
  2. The create-pull-request step in the create-pr-to-sync-owners task requires several inputs that must be manually added each time a new team is created. It would be better to automate the process of adding new teams to the task.
  3. The filter on repo owner and name interceptor in the branch-create-product-components-hotfix trigger filters only on the repository owner and name. It would be better to also filter on the action field to ensure that the trigger only fires on branch creation events.

To address these issues, I suggest the following changes:

  1. Add a check to the clone-community-repo step to ensure that the repository is not already present before cloning it.
  2. Modify the create-pull-request step to dynamically generate the list of team inputs based on the contents of the teams directory in the community repository.
  3. Modify the filter on repo owner and name interceptor to also filter on the action field, using a value of created to ensure that the trigger only fires on branch creation events.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Oct 25, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.
Summary:

This pull request adds functionality to automatically create pull requests when a hotfix branch is created. The changes include the addition of a new task to create pull requests, a new trigger template to create hotfix branches and a change to the github-branch-push.yaml file to include the correct git-ref parameter.

Potential Problems:

  1. Security: The script in the create-pr-to-sync-owners.yaml file includes the use of a GitHub private token. It is important to ensure that this token is being handled securely and not being exposed in any way.
  2. Testing: It is not clear if this code has been tested thoroughly. It is important to ensure that the new functionality works as expected and does not introduce any new bugs.
  3. Code Quality: There are a number of minor formatting issues in the code, such as inconsistent indentation and extra whitespace. It is important to maintain a consistent coding style to make the code more readable and easier to maintain.

Fixing Suggestions:

  1. Security: Consider using a secret to store the GitHub private token instead of hardcoding it into the script. This will help keep the token secure and prevent it from being accidentally exposed.
  2. Testing: Ensure that the new functionality is tested thoroughly before merging the code. Consider writing unit tests and integration tests to verify the behavior of the new code.
  3. Code Quality: Use a linter or code formatter to help maintain consistent formatting throughout the code. This will make the code more readable and easier to maintain.

Copy link
Contributor

@purelind purelind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Oct 25, 2023

[LGTM Timeline notifier]

Timeline:

  • 2023-10-25 03:15:43.672251499 +0000 UTC m=+2404541.259361628: ☑️ agreed by purelind.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Oct 25, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: purelind

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot added the approved label Oct 25, 2023
@ti-chi-bot ti-chi-bot bot merged commit 46653f0 into main Oct 25, 2023
4 checks passed
@ti-chi-bot ti-chi-bot bot deleted the feature/add-tekton-jobs-to-sync-owners-files-to-new-hotfix-branches branch October 25, 2023 03:17
wuhuizuo added a commit that referenced this pull request Oct 26, 2023
…WNERS for hotfix branches (#737)

* fix(apps/prod/tekton/configs/triggers/bindings): fix branch and tag bindings

- tag creating event
- branch creating event
- branch pushing event

Signed-off-by: wuhuizuo <wuhuizuo@126.com>

* Update apps/prod/tekton/configs/triggers/bindings/github-branch-create.yaml

* feat(apps/prod/tekton/configs): add task and triggers for auto sync OWNERS for hotfix branches

Automatically create pull requests when hot fix branch is created.

Signed-off-by: wuhuizuo <wuhuizuo@126.com>

* Update apps/prod/tekton/configs/triggers/templates/branch-create-product-components-hotfix.yaml

* Update apps/prod/tekton/configs/triggers/triggers/create-branch-product-components-hotfix.yaml

* Update apps/prod/tekton/configs/triggers/triggers/create-branch-product-components-hotfix.yaml

---------

Signed-off-by: wuhuizuo <wuhuizuo@126.com>
wuhuizuo added a commit that referenced this pull request Oct 26, 2023
…WNERS for hotfix branches (#737)

* fix(apps/prod/tekton/configs/triggers/bindings): fix branch and tag bindings

- tag creating event
- branch creating event
- branch pushing event

Signed-off-by: wuhuizuo <wuhuizuo@126.com>

* Update apps/prod/tekton/configs/triggers/bindings/github-branch-create.yaml

* feat(apps/prod/tekton/configs): add task and triggers for auto sync OWNERS for hotfix branches

Automatically create pull requests when hot fix branch is created.

Signed-off-by: wuhuizuo <wuhuizuo@126.com>

* Update apps/prod/tekton/configs/triggers/templates/branch-create-product-components-hotfix.yaml

* Update apps/prod/tekton/configs/triggers/triggers/create-branch-product-components-hotfix.yaml

* Update apps/prod/tekton/configs/triggers/triggers/create-branch-product-components-hotfix.yaml

---------

Signed-off-by: wuhuizuo <wuhuizuo@126.com>
wuhuizuo added a commit that referenced this pull request Oct 26, 2023
…WNERS for hotfix branches (#737)

* fix(apps/prod/tekton/configs/triggers/bindings): fix branch and tag bindings

- tag creating event
- branch creating event
- branch pushing event

Signed-off-by: wuhuizuo <wuhuizuo@126.com>

* Update apps/prod/tekton/configs/triggers/bindings/github-branch-create.yaml

* feat(apps/prod/tekton/configs): add task and triggers for auto sync OWNERS for hotfix branches

Automatically create pull requests when hot fix branch is created.

Signed-off-by: wuhuizuo <wuhuizuo@126.com>

* Update apps/prod/tekton/configs/triggers/templates/branch-create-product-components-hotfix.yaml

* Update apps/prod/tekton/configs/triggers/triggers/create-branch-product-components-hotfix.yaml

* Update apps/prod/tekton/configs/triggers/triggers/create-branch-product-components-hotfix.yaml

---------

Signed-off-by: wuhuizuo <wuhuizuo@126.com>
wuhuizuo added a commit that referenced this pull request Oct 26, 2023
…WNERS for hotfix branches (#737)

* fix(apps/prod/tekton/configs/triggers/bindings): fix branch and tag bindings

- tag creating event
- branch creating event
- branch pushing event

Signed-off-by: wuhuizuo <wuhuizuo@126.com>

* Update apps/prod/tekton/configs/triggers/bindings/github-branch-create.yaml

* feat(apps/prod/tekton/configs): add task and triggers for auto sync OWNERS for hotfix branches

Automatically create pull requests when hot fix branch is created.

Signed-off-by: wuhuizuo <wuhuizuo@126.com>

* Update apps/prod/tekton/configs/triggers/templates/branch-create-product-components-hotfix.yaml

* Update apps/prod/tekton/configs/triggers/triggers/create-branch-product-components-hotfix.yaml

* Update apps/prod/tekton/configs/triggers/triggers/create-branch-product-components-hotfix.yaml

---------

Signed-off-by: wuhuizuo <wuhuizuo@126.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/apps env/prod will deploy on the main product cluster lgtm size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants