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

Fix comparison logic and snyk stage naming conventions #11545

Merged
merged 2 commits into from
Jan 3, 2024
Merged

Conversation

berryd
Copy link
Contributor

@berryd berryd commented Jan 2, 2024

Description

This PR will be identical for all repositories.

MACPRO took an approach of relaying all Snyk generated builds into a single ephemeral environment. This is accomplished by setting the number of PRs Snyk can create outstanding to be one per project, and by setting a concurrency group in GHA workflow to ensure only one Synk build can occur at a time. This makes sense for keeping infrastructure costs down, but it creates the situation where builds don't match the health of the environment, such that a successful build after a failed one will rectify the environment. This is probably not much of a concern, but it's worth pointing out they are using OpenSearch and significantly more expensive infrastructure, and that limiting the creation of Snyk generated branches is only applicable at a service (package.json) level, so instead, I took the approach of correcting the environment switch such that Snyk will generate one build per branch, but I link these builds against the Synk hash and truncate the branch name to avoid failures.

Dependabot branches are done much in the same way, where an m5sum is created from the branch name and truncated to 10 characters, with an 'x' appended, such that the result will be x0000000000. The previous fix put in place doesn't correctly capture the branch name and failed the check, and would attempt to create Cloudformation templates using the full Synk generated branch name. Instead I've put in a basic regular expression that greedily matches snyk-fix-xxx and snyk-upgrade-xxx, but I'm stripping the first 10 characters from the hash Snyk appends to the name in order to easily link a branch to the created infrasturcture, such that the branch name snyk-upgrade-02ff80f020e0ac03d1ada0575a20f214 will appears as s02ff80f020 as a stack namespace.

Implementing this fix requires a few steps, manual in nature:

  1. Firstly, this PR for each repository needs to be merged into the main branch and the deploy to succeed.
  2. Each repository will have a number of Snyk generated branches that are sitting on top of failed to deploy infrastructure. These branches all need to be pulled down to the engineer's workstation to complete the action.
  3. The remote branches created by Snyk need to all be destroyed.
  4. The main branch now completed with the fix needs to be updated on the developer's workstation, and merged into each Snyk generated branch.
  5. Each Snyk generated branch can be pushed back to origin.
  6. Finally each PR created by Snyk, which was closed by deleting the remote branches, can be reopen.

Some changes to Snyk configuration were made during the development of this work, but those changes have been reversed.

Related ticket(s)

CMDCT-3081


How to test

This PR is essentially a no-op for a normal build path. Successful application will be demonstrated by Snyk generated branches being able to successfully build infrastructure and pass tests.

Important updates

The behavior of Dependabot has not been altered but it is reasonable to remove it from the toolset given the overlap against Snyk. The changes are very minor and mainly related to correcting parsing of the branch references.


Author checklist

  • I have performed a self-review of my code
  • I have added thorough tests, if necessary
  • I have updated relevant documentation, if necessary

convert to a different template: test → val | val → prod

@berryd berryd self-assigned this Jan 2, 2024
Copy link

codeclimate bot commented Jan 2, 2024

Code Climate has analyzed commit fb239ff and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (90% is the threshold).

This pull request will bring the total coverage in the repository to 95.8% (0.0% change).

View more on Code Climate.

@berryd berryd merged commit e23abdb into main Jan 3, 2024
17 checks passed
@berryd berryd deleted the fix-snyk-stages branch January 3, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants