-
Notifications
You must be signed in to change notification settings - Fork 15
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
refactor: CI/CD Pipelines #259
Conversation
@@ -19,6 +18,7 @@ env: | |||
jobs: | |||
prerequisites: | |||
name: "Prerequisites" | |||
if: ${{ github.event_name == 'workflow_dispatch' || (github.event_name == 'pull_request_review' && github.event.review.state == 'approved' && startsWith(github.event.pull_request.head.ref, 'RELEASE-')) }} |
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.
Discussion
We are still having a pull_request_review
trigger here. So even though the job would be skipped for any PR that is not coming from a RELEASE-*
branch, it would still appear within the status checks of every other PR.
So maybe we should get rid of the automatic trigger altogether?
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.
would agree, one extra button press isn't the world, better not to have our PR checks (even more) cluttered.. no strong opinion though ofc...
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.
I removed the automatic trigger and added another checkbox to the release PR body (i.e. Trigger the Perform Release Workflow).
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.
mostly LGTM, one minor question on the snapshot deployment
didn't look at the release notes python yet
description: "The username to use when authenticating with the repository." | ||
required: true | ||
repository-password: | ||
description: "The password to use when authenticating with the repository." |
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.
Not too sure if this is safe to do. Maybe input parameters are logged in plaintext somewhere? Maybe just pass the name of the secret instead?
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.
Generally, it seems to be a "best practice" to pass around secrets like this
(see the token
parameter of the checkout action for example).
But I'd still agree: As this is our own action, we don't need to pass around the actual secrets. Using the secret names should be sufficient 👍
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.
Unfortunately, passing the secret names (instead of their actual values), doesn't work as it would require some sort of "nested GitHub macro extension" like so:
- run: something
env:
GH_TOKEN: ${{ secrets.${{ inputs.secret-name}} }}
This doesn't seem to be supported by GH actions.
comments have been addressed.
Context
SAP/cloud-sdk-java-backlog#318
Continuation of #235
This PR refactors our GitHub actions to, first and foremost, avoid the massive code duplication.
Additionally, it also refactors the release flow, which now provides a lot more automation and should be easier to use in general.
How CI Works
The CI portion of our pipelines has been changed in the following ways:
continuous-integration.yaml
).main
branch) receives an update.main-build.yaml
, ourprepare-release.yaml
, and ourdeploy-snapshot.yaml
). This is how we are getting rid of the code duplication.How CD Works
The CD portion (i.e. performing releases) has also been changed.
Here is the new flow:
Prepare Release
(prepare-release.yaml
) workflow. There are sensitive default parameters in place, so no manual input is required.5.3.0
)continuous-integration.yaml
workflow on the release commit. This includes code checks, such as Black Duck, CodeQL, etc.rel/<VERSION>
) and also create a new draft release in our GitHub repoPrepare Release
workflow succeeds, the release facilitator will find a new PR with detailed instructions about what to do in our Code Repo (i.e.SAP/cloud-sdk-java
). These TODOs include the following:release-notes.md
)Perform Release
workflow will be triggered automatically.Perform Release
workflow can also be triggered manually now.