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

Enable actions if code scanning is enabled? #48

Merged
merged 5 commits into from
Mar 24, 2023

Conversation

joshjohanning
Copy link
Contributor

I ran into a situation where I had imported/migrated a couple of repositories and actions were disabled by default, so when the the code scanning workflow file was pushed, the workflow never ran.

Would it make sense to attempt to enable Actions on the repositories it pushes a code scanning file? It adds another API call, not sure if worth it or not. Perhaps it would have other unintended consequences.

I debated on whether submitting this as a PR or not; feel free to reject/suggest changes. Perhaps there's some middle ground where maybe this is just another option/flag on the run.

}
);

if (status !== 204) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if actions are already enabled? 🤔

Does this throw a 204 even if actions are already enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, 204 even if actions are already enabled.

@NickLiffen
Copy link
Owner

Thanks for submitting the PR 👍

Overall, I am good with this, but it is going to be "another API call", and I am cautious about adding more API calls.

Are there any GraphQL API calls to do this? 🤔

I am trying to think of a more efficient way of doing this 👁️ 🤔

tl;dr I wanted to clean up a few of the API calls being made here, as a few of them are wasted. E.G it annoys me that we first don't make a graphql API call to get information about the repository, e.g.

  • Is GHAS already enabled?
  • Is secret scanning already enabled?
  • Is dependabot enabled?

I almost want to make a GET request which checks to see if these are already enabled, and only make the POST or PATCH request if these things aren't enabled.

The problem I see is people want to enable GHAS, secret scanning on 1000 repos, and when they run the script 300 of them already have everything enabled. It's such a wasted API call to go and enable things again 🤔

So I think a GET request first may be better 🤔

The reason why I say this is because the above would fall into this category 💯

I am also trying to move my API calls to GraphQL 🙃

Just wanted to share my thoughts 👀 not saying no to this, just wanted to share my thoughts.

@joshjohanning
Copy link
Contributor Author

Totally on the same page here!

Overall, I am good with this, but it is going to be "another API call", and I am cautious about adding more API calls.

Totally - running with a GitHub app should help with large runs but might still run up against the rate limit.

Are there any GraphQL API calls to do this? 🤔

Not that I could find 😢

I almost want to make a GET request which checks to see if these are already enabled, and only make the POST or PATCH request if these things aren't enabled.

This seems like the preferred route - but wouldn't that result in possibly an additional API call? (up to 2 calls per repo, minimum 1, vs 1?)

Copy link

@wdos455 wdos455 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How to purchase

@benhorgen
Copy link

I think this issue is closed, right? I thought Action are being enabled with the script in it's current state.

@NickLiffen
Copy link
Owner

@joshjohanning I am thinking about getting this merged based on some feedback; when you get a second, is there any chance we could maybe get this back alive again 🙇

@joshjohanning
Copy link
Contributor Author

I'll take a look! 👀

@joshjohanning
Copy link
Contributor Author

I think this issue is closed, right? I thought Action are being enabled with the script in it's current state.

I am thinking about getting this merged based on some feedback; when you get a second, is there any chance we could maybe get this back alive again 🙇

@benhorgen @NickLiffen super long delay. I still think this is a (small) issue, and maybe more related to how I was testing this, but it's possible depending on how people have their GitHub repositories set up or migrated in, they could have the same issue.

Repro steps:

  1. Use the import repo feature
  2. Run this tool to have a codeql workflow file created
  3. Merge in the PR - the CodeQL workflow won't run because by default, actions aren't enabled (disabled by default on repository imports)

I did test this code again, and it does still work by creating actions (see the actions line in the log below)

  ghas:inform Octokit Client Generated +0ms
  ghas:inform Currently looping over: 1/1. The org name is: joshjohanning-ghas-enablement +0ms
  ghas:inform Currently looping over: 1/1. The repo name is: joshjohanning-ghas-enablement/webgoat-import +0ms
  ghas:inform Enabled GHAS for webgoat-import. Status: 200 +898ms
  ghas:inform Is Dependabot Alerts enabled already for webgoat-import? : Enabled +539ms
  ghas:inform Enabled Dependabot Security Updates for webgoat-import. Status: 204 +669ms
  ghas:inform Enabled Secret Scanning for webgoat-import. Status: 200 +733ms
  ghas:inform Actions enabled on the following repository webgoat-import +1s

@joshjohanning
Copy link
Contributor Author

I changed this to be less prescriptive (ie: automatically enable actions if code scanning is enabled) and instead broke it out to a separate switch.

Is this more preferred? What do we think?

ENABLE_ON=actions,codescanning,secretscanning,dependabot,dependabotupdates,pushprotection
[
  {
    "login": "joshjohanning-ghas-enablement",
    "repos": [
      {
        "enableDependabot": true,
        "enableDependabotUpdates": true,
        "enableSecretScanning": true,
        "enableCodeScanning": true,
        "enablePushProtection": true,
        "enableActions": true,
        "primaryLanguage": "javascript",
        "createIssue": true,
        "repo": "joshjohanning-ghas-enablement/webgoat-import-2"
      }
    ]
  }
]

Not sure if we want to make the switch something more specific like turnOnActions -- It won't disable actions if you enableActions set to false, it just won't change anything related to actions.

@NickLiffen NickLiffen self-requested a review March 21, 2023 11:59
@NickLiffen
Copy link
Owner

@joshjohanning I like this approach and sounds good 🙇

@joshjohanning
Copy link
Contributor Author

@NickLiffen great, anything else we want to check/test/verify/review before merging (I think the button is on you 😄 ⏯️ )

@NickLiffen NickLiffen merged commit c7b2a11 into NickLiffen:main Mar 24, 2023
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.

4 participants