Skip to content

[SYSE-336 master] Followup from March template application#6207

Merged
alephnull merged 2 commits into
masterfrom
releng/master
Apr 8, 2024
Merged

[SYSE-336 master] Followup from March template application#6207
alephnull merged 2 commits into
masterfrom
releng/master

Conversation

@alephnull

@alephnull alephnull commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

Learning from the application:
Applying templates across all branches and repos runs up against the usage limits. Larger runners are available for Team plans. This needs to be explored.
A local run of release.yml is needed to test updates before pushing it to all repos and waiting for >30mins to find out if it works.
For regular updates which collect disparate code changes, the git commit log in the repos should reflect a summary of changes that went into it.

  • Mount go package and build cache into build container
  • nektos/act for local runs
  • dependabot alerts @devops for only .g/w/release.yml
  • r5-lts to test against pump and sink master
  • Larger runners for Actions
  • pk for byol build
  • Converge UI and API tests compose environments in tyk-ci

@alephnull
alephnull requested a review from a team as a code owner April 8, 2024 06:08
@alephnull
alephnull enabled auto-merge (squash) April 8, 2024 06:08
@github-actions

github-actions Bot commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

API Changes

no api changes detected

@github-actions

github-actions Bot commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

PR Description updated to latest commit (71fe9ea)

@github-actions

github-actions Bot commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

PR Review

⏱️ Estimated effort to review [1-5]

2, because the PR mainly involves removing timestamp comments and minor adjustments in scripts and configuration files. The changes are straightforward and do not introduce complex logic or new features.

🧪 Relevant tests

No

🔍 Possible issues

Possible Bug: The removal of timestamp comments is unlikely to introduce bugs, but the changes in .github/workflows/release.yml related to environment variables and paths could potentially affect the build process if not thoroughly tested.

🔒 Security concerns

No

Code feedback:
relevant file.github/workflows/release.yml
suggestion      

Consider using a more descriptive variable name than NFPM_STD_PASSPHRASE. Descriptive names improve code readability and maintainability. [medium]

relevant line-e NFPM_STD_PASSPHRASE="$NFPM_STD_PASSPHRASE" \

relevant file.github/workflows/release.yml
suggestion      

Ensure that the changes in spacing and the introduction of GOCACHE and GOMODCACHE environment variables do not unintentionally alter the behavior of the build process. It might be beneficial to add a comment explaining the reason for these changes for future reference. [medium]

relevant line-e GOCACHE=/cache/go-build \

relevant file.github/workflows/release.yml
suggestion      

The change from override to overwrite in the artifact upload steps improves clarity. Ensure that all relevant documentation and related scripts are updated to reflect this terminology change to maintain consistency across the project. [medium]

relevant lineoverwrite: true

relevant file.github/workflows/release.yml
suggestion      

Verify that the introduction of explicit paths for GOMODCACHE and GOCACHE does not conflict with any existing caching mechanisms or paths used in other parts of the workflow or in related workflows. Consistency in cache handling is crucial for efficient CI/CD operations. [medium]

relevant line-v ~/.cache/go-build:/cache/go-build \


✨ Review tool usage guide:

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

  • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

@github-actions

github-actions Bot commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

PR Code Suggestions

CategorySuggestions                                                                                                                                                       
Maintainability
Improve variable name for better readability.

Consider using a more descriptive variable name than NFPM_STD_PASSPHRASE to enhance code
readability and maintainability. A name that reflects its purpose or usage context could
make the codebase more understandable to new contributors or maintainers.

.github/workflows/release.yml [94]

--e NFPM_STD_PASSPHRASE="$NFPM_STD_PASSPHRASE"
+-e PACKAGE_SIGNING_PASSPHRASE="$PACKAGE_SIGNING_PASSPHRASE"
 
Extract environment variables and volume mounts for better maintainability.

To improve the maintainability of the workflow, consider extracting the environment
variables and volume mounts into reusable GitHub Actions environment files or scripts.
This can make the workflow easier to read and modify.

.github/workflows/release.yml [89-98]

--e GOPRIVATE=github.com/TykTechnologies
--v ${{github.workspace}}:/go/src/github.com/TykTechnologies/tyk
+env: .github/workflows/envs/build-envs
+volumes: .github/workflows/volumes/build-volumes
 
Security
Use GitHub Secrets for sensitive environment variables.

Ensure that sensitive environment variables like GPG_FINGERPRINT and PKG_SIGNING_KEY are
not hardcoded in the workflow file. Use GitHub Secrets to securely manage sensitive
information.

.github/workflows/release.yml [95-96]

--e GPG_FINGERPRINT=12B5D62C28F57592D1575BD51ED14C59E37DAC20
--e PKG_SIGNING_KEY="$PKG_SIGNING_KEY"
+-e GPG_FINGERPRINT=${{ secrets.GPG_FINGERPRINT }}
+-e PKG_SIGNING_KEY=${{ secrets.PKG_SIGNING_KEY }}
 
Best practice
Remove or consolidate overlapping volume mounts.

To avoid potential conflicts and ensure a clean build environment, consider removing or
consolidating the volume mounts that overlap, such as /go/pkg/mod and /cache/go-build.

.github/workflows/release.yml [103-104]

--v ~/go/pkg/mod:/go/pkg/mod
--v ~/.cache/go-build:/cache/go-build
+-v /go/pkg/mod:/go/pkg/mod
+-v /cache/go-build:/cache/go-build
 
Possible issue
Correct the artifact overwrite option in GitHub Actions workflow.

The overwrite: true option in the actions/upload-artifact@v4 step is correctly intended to
replace existing artifacts, but the correct option is if-no-files-found: 'overwrite'
according to the latest GitHub Actions documentation.

.github/workflows/release.yml [277-285]

-overwrite: true
+if-no-files-found: 'overwrite'
 

✨ Improve tool usage guide:

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

  • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

@github-actions

github-actions Bot commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

@github-actions

github-actions Bot commented Apr 8, 2024

Copy link
Copy Markdown
Contributor

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

@alephnull
alephnull merged commit 7081653 into master Apr 8, 2024
@sonarqubecloud

sonarqubecloud Bot commented Apr 8, 2024

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
50.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@alephnull
alephnull deleted the releng/master branch April 8, 2024 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants