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: Ensure exemption list of ecosystem is not overwritten #216

Merged
merged 6 commits into from
Sep 7, 2024

Conversation

hkadakia
Copy link
Contributor

@hkadakia hkadakia commented Sep 6, 2024

Pull Request

Proposed Changes

Currently we pass in the env variable directly when building a dependabot config file. When iterating through multiple repos, there is a possibility that if one of the previous repos has an existing dependabot config, we land up adding those to the exemption list in order to not add that config again to the dependabot.yml file. We land up using the same exempt_ecosystem for all the foll. repos resulting in skipping of creating that dependabot.yml.

This fix always passes in a copy of the original exempt_ecosystems environment variable ensuring the original data is not over written.

Checking hello-gh-actions for compatible package managers
before adding exempt_ecosystems:  []
existing_config:  <Contents [.github/dependabot.yaml]>
 exempt_ecosystems:  ['gomod', 'docker', 'github-actions']
repo: hello-gh-actions manager: bundler
repo: hello-gh-actions manager: npm
repo: hello-gh-actions manager: pip
repo: hello-gh-actions manager: cargo
repo: hello-gh-actions manager: gomod
repo: hello-gh-actions manager: composer
repo: hello-gh-actions manager: mix
repo: hello-gh-actions manager: nuget
repo: hello-gh-actions manager: docker
	No (new) compatible package manager found
Checking test for compatible package managers
before adding exempt_ecosystems:  ['gomod', 'docker', 'github-actions']
existing_config:  None
 exempt_ecosystems:  ['gomod', 'docker', 'github-actions']
repo: test manager: bundler
repo: test manager: npm
repo: test manager: pip
repo: test manager: cargo
repo: test manager: gomod
repo: test manager: composer
repo: test manager: mix
repo: test manager: nuget
repo: test manager: docker
	No (new) compatible package manager found

In the example above, the first repo hello-gh-actions already has a dependabot.yml file with gomod, docker & github-actions package manager. The test repo on the other hand does not have any dependabot.yml file and the expectation is for evergreen to create a pull request but as you see from the example it assumes the same ecosystems are added to the exempt list and skips adding the pull request.

Below is my evergreen.yml config:

 - name: Run evergreen action
    uses: github/evergreen@ff2f3ffd6bf88d33354f49a79ecbffd1d0cf98c9 #v1.12.0
    env:
      GH_TOKEN: ${{ secrets.GH_TOKEN }}
      ORGANIZATION: <orgid>
      TITLE: "Add dependabot configuration"
      TYPE: pull
      BATCH_SIZE: 10
      ENABLE_SECURITY_UPDATES: true
      UPDATE_EXISTING: true

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • run make lint and fix any issues that you have introduced
  • run make test and ensure you have test coverage for the lines you are introducing
  • If publishing new data to the public (scorecards, security scan results, code quality results, live dashboards, etc.), please request review from @jeffrey-luszcz

Reviewer

  • Label as either fix, documentation, enhancement, infrastructure, maintenance or breaking

@github-actions github-actions bot added the fix label Sep 6, 2024
@hkadakia hkadakia marked this pull request as ready for review September 6, 2024 18:06
@jmeridth
Copy link
Member

jmeridth commented Sep 6, 2024

@hkadakia thank you for catching this. Would you be willing to add a test? I see your manual testing. Thank you for that. Gave great context.

@hkadakia
Copy link
Contributor Author

hkadakia commented Sep 6, 2024

@hkadakia thank you for catching this. Would you be willing to add a test? I see your manual testing. Thank you for that. Gave great context.

Sure. I'll work on it as soon as I can.

@hkadakia
Copy link
Contributor Author

hkadakia commented Sep 6, 2024

@jmeridth Added the unit test. In order to ensure we correctly test/exercise the code, moved the copy from the calling function to within the function. Please let me know if any more changes.

@jmeridth
Copy link
Member

jmeridth commented Sep 7, 2024

@hkadakia some linting errors. once fixed I'm good with merging this. Thank you very much for the contribution including tests.

@jmeridth jmeridth merged commit 3d7db4c into github:main Sep 7, 2024
6 checks passed
@zkoppert
Copy link
Member

zkoppert commented Sep 7, 2024

Wow! Big learning opportunity for me here. Thanks for finding and fixing this.

I'm adding "overwriting env data when passing and returning to functions" to my personal code review checklist as I think this is something I could easily do again if I'm not careful about it.

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.

3 participants