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

chore(prod/jenkins): add secret for ks3util config #1353

Conversation

purelind
Copy link
Contributor

Add secret for ks3util config.

Already test in jenkins beta: #1352

@ti-chi-bot ti-chi-bot bot requested a review from wuhuizuo November 27, 2024 08:42
@ti-chi-bot ti-chi-bot bot added area/apps env/prod will deploy on the main product cluster labels Nov 27, 2024
Copy link
Contributor

ti-chi-bot bot commented Nov 27, 2024

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

Based on the PR title and description, it seems that the author is adding a secret for ks3util configuration in the production Jenkins environment. The changes are made in the values-JCasC.yaml file of the prod/jenkins folder.

The changes add a new secret for ks3util configuration with the name ks3util-config. The secret is added to the domainCredentials section of the system section. The secret is defined as a file with a fileName of ks3util.conf and secretBytes set to ${ks3util-config}.

It seems that the changes are straightforward and there are no obvious problems with the code. However, there are a few suggestions that can be made to improve the PR:

  • It is always a good practice to include a brief explanation of why the changes are being made in the PR description. This can help reviewers and future developers understand the context of the changes.

  • Although the changes have been tested in the Jenkins beta environment, it is still a good idea to run the changes through some automated tests before merging them into the production environment. This can help catch any potential issues that may arise.

  • It is important to ensure that the secret is properly secured and only accessible to authorized users. The PR description does not mention any details about the access control for the secret, so it may be a good idea to double-check the access control settings.

Overall, the changes seem fine, and the suggestions are minor. The PR can be approved and merged into the production environment after addressing the suggestions.

@ti-chi-bot ti-chi-bot bot added the size/XS label Nov 27, 2024
@purelind
Copy link
Contributor Author

/hold

Effective when there is no pipelines activity.

Copy link
Contributor

ti-chi-bot bot commented Dec 1, 2024

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

@purelind
Copy link
Contributor Author

purelind commented Dec 1, 2024

/unhold

@ti-chi-bot ti-chi-bot bot merged commit 1bf290b into PingCAP-QE:main Dec 1, 2024
4 checks passed
purelind added a commit that referenced this pull request Dec 1, 2024
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/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant