Skip to content

Conversation

@mamercad
Copy link
Contributor

@mamercad mamercad commented Dec 15, 2022

Add a Workflow which ensure that the "Build and Test" Workflows are enabled in all of StackStorm-Exchange; GitHub will disable all Workflows in a repository if 60d have gone by without activity; fixes #136.

@mamercad mamercad changed the title Add Enable Pack Release Workflow Add "Enable Pack Release" Workflow Dec 16, 2022
Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

I understand this is a draft. Thanks for tackling this!

Here are a couple of thoughts.

Would you document (probably in a comment) what permissions are required to use this? What permissions should ${{ github.token }} (or equivalent) have?

I suspect we will need to pass a token in as a secret. I, or one of the senior maintainers, can help setup that token so it can be accessed via GHA workflows.

@mamercad mamercad marked this pull request as ready for review December 16, 2022 23:51
@mamercad mamercad requested a review from cognifloyd December 17, 2022 00:23
@mamercad mamercad requested review from arm4b and removed request for cognifloyd December 17, 2022 16:47
@mamercad mamercad changed the title Add "Enable Pack Release" Workflow Add "Enable Pack Build and Test" Workflow Dec 17, 2022
Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

Looking good. Next time I have a moment I will make the PAT.

Does the user who makes the PAT need to be an admin? If not, I'll use the stackstorm-neptr bot account.

@arm4b
Copy link
Member

arm4b commented Dec 17, 2022

From the StackStorm-Exchange/index#27 (comment)

I wonder if it would make more sense to schedule this workflow on https://github.com/StackStorm-Exchange/ci instead of on the index. The current workflow in this repo only has write access to the index repo, and nothing else. I think I'd like to keep it that way.

@cognifloyd @mamercad I think the corner case here is that this repository GH Workflows can expire the same way as packs, making this workflow non-running after the 60 days.

With that, the difference between this repo and Index (https://github.com/StackStorm-Exchange/index/) is that Index repo CI won't expire due to daily automated commits.

🤔

@mamercad
Copy link
Contributor Author

From the StackStorm-Exchange/index#27 (comment)

I wonder if it would make more sense to schedule this workflow on https://github.com/StackStorm-Exchange/ci instead of on the index. The current workflow in this repo only has write access to the index repo, and nothing else. I think I'd like to keep it that way.

@cognifloyd @mamercad I think the corner case here is that this repository GH Workflows can expire the same way as packs, making this workflow non-running after the 60 days.

With that, the difference between this repo and Index (https://github.com/StackStorm-Exchange/index/) is that Index repo CI won't expire due to daily automated commits.

🤔

Ah yes, thinking... we found the "zero point" 😄

echo "::group::Ensure ${WORKFLOW_NAME} workflow for ${PACKS_ORG}/${repo_name} is active"
if gh api --silent --method GET ${WORKFLOW_PATH} 2>/dev/null; then
echo "GitHub ${WORKFLOW_NAME} workflow found for ${PACKS_ORG}/${repo_name}"
if [[ "$(gh api --method GET ${WORKFLOW_PATH} --jq .state)" != "active" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

What if we skipped this check and just blindly enable it every time this workflow runs? That way, the 60d clock would reset every time this runs.

I wonder if it could also self-enable? So, every time it runs it hits the API to reset the clock on the CI repo as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, that's interesting and might work, have you found docs around what things specifically constitute repository activity? I have not ... well, other than the obvious (commits ... and I'm with you, I don't like dummy commits).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wish we knew what types of things constituted "repository activity" ... I still haven't managed to find specific information on this. I'm hoping it's more than just "commits against the main branch" (because that'd mean we'd have to do dummy commits into it). What's the monitoring story like for StackStorm, maybe an alternative to dummy commits into master would be to simply monitor this job's enabled or disabled status with a "daily dead man's switch" or something along those lines.

@mamercad
Copy link
Contributor Author

Looking good. Next time I have a moment I will make the PAT.

Does the user who makes the PAT need to be an admin? If not, I'll use the stackstorm-neptr bot account.

I don't think it does.

@mamercad
Copy link
Contributor Author

Looking good. Next time I have a moment I will make the PAT.
Does the user who makes the PAT need to be an admin? If not, I'll use the stackstorm-neptr bot account.

I don't think it does.

Also, let me know if/when you're ready to merge this, I can rebase it beforehand.

Comment on lines +4 to +6
# Daily at 3 in the morning
schedule:
- cron: "0 3 * * *"
Copy link
Member

Choose a reason for hiding this comment

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

OK. I created the secret. I'm almost ready to merge this.

@armab I think once a month is enough, assuming that the API call resets the 60 day timer. That assumption is also baked into this:
https://stackoverflow.com/a/68591566/1134951
https://github.com/invenia/KeepActionsAlive

So, can we try once a month, and then if that turns out not to work, we can tweak this?

Copy link
Member

@arm4b arm4b Dec 18, 2022

Choose a reason for hiding this comment

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

What if we skipped this check and just blindly enable it every time this workflow runs? That way, the 60d clock would reset every time this runs.

I hope it'll work, but I'd doubt it's possible to re-enable not yet disabled GH workflow, considering the solution mentioned there and the code: https://github.com/invenia/KeepActionsAlive/blob/main/src/app.py#L73

Otherwise, we'd see similar solutions in the wild when the github repo could re-enable itself, instead of using the https://github.com/marketplace/actions/keepalive-workflow self-commit approach.

More than that, the GH Actions may expire after 2 months in this repo too. With that, I think there's a need to use this workflow in the https://github.com/stackStorm-exchange/index, as we have automated commits in there and confidence it won't expire.

But we can try as is in this repo and see how it goes to verify our guesses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if we skipped this check and just blindly enable it every time this workflow runs? That way, the 60d clock would reset every time this runs.

I hope it'll work, but I'd doubt it's possible to re-enable not yet disabled GH workflow, considering the solution mentioned there and the code: https://github.com/invenia/KeepActionsAlive/blob/main/src/app.py#L73

Otherwise, we'd see similar solutions in the wild when the github repo could re-enable itself, instead of using the https://github.com/marketplace/actions/keepalive-workflow self-commit approach.

More than that, the GH Actions may expire after 2 months in this repo too. With that, I think there's a need to use this workflow in the https://github.com/stackStorm-exchange/index, as we have automated commits in there and confidence it won't expire.

But we can try as is in this repo and see how it goes to verify our guesses.

Yeah, my suspicion is that this "self-enabling" approach won't work either; using the index isn't a terrible approach since the index and ci repositories are functionally coupled anyhow, right?

Copy link
Member

Choose a reason for hiding this comment

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

The index repo would be more convenient, true. But, I want to avoid adding a PAT on the index repo.

Right now, all the packs get pulled into the index CI. If someone compromises one of our packs, then the most they can do is mess up the index, they can't use the index to get to any other exchange packs.

If we add a PAT with write access to repos and workflows, then that widens such an attack vector. Then, if someone is able to compromise a pack and the index, they could jump from there to all the packs.

This CI repo is used in workflows across the exchange. Anyone that gets access to this repo already has access to all the packs. But, gaining access to any of the packs does not give any attack vector that could compromise this repo. So, I would prefer to have the PAT only on this repo.

(Hmm, maybe this workflow should avoid cloning all the packs, and rely on the GitHub API to look up everything about the available workflows).

If the API call doesn't work on this repo (I agree, there's a good chance it won't), then we can do one of three things:

  • add dummy commits (doing it only in this repo, while ugly, is better than on all of the pack repos)
  • add a workflow in st2 ci that deals with this repo (and only this repo, not the rest of the exchange)
  • add a Circle I workflow that keeps this repo active.

I kind of like the idea of using CircleCI here. The token we add to CircleCI could use a fine grained token so that it can only hit the workflow enable endpoint on this one repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we add a PAT with write access to repos and workflows

I don't think this would need write on the repos though, just on the workflows.

Copy link
Member

Choose a reason for hiding this comment

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

All the good options 👍

  • fine-grained PAT
  • CircleCI
  • automated commits in this repo

I'm fine either way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, deferring to @cognifloyd on which way to go!

Copy link
Member

Choose a reason for hiding this comment

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

I'm merging it as is. Let's see how things go and then make changes in follow-ups

Comment on lines +4 to +6
# Daily at 3 in the morning
schedule:
- cron: "0 3 * * *"
Copy link
Member

Choose a reason for hiding this comment

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

I'm merging it as is. Let's see how things go and then make changes in follow-ups

@cognifloyd cognifloyd merged commit 5a188a3 into StackStorm-Exchange:master Dec 20, 2022
@cognifloyd
Copy link
Member

@mamercad looks like there are some gotchas with the workflow. Could you take a look? I suspect we need to just change the working directory, or specify the repo on the command line:
https://github.com/StackStorm-Exchange/ci/actions/runs/3737834951/jobs/6343366911

@mamercad
Copy link
Contributor Author

@mamercad looks like there are some gotchas with the workflow. Could you take a look? I suspect we need to just change the working directory, or specify the repo on the command line: https://github.com/StackStorm-Exchange/ci/actions/runs/3737834951/jobs/6343366911

Surely; the first thing that I've noticed is that it looks like this is a thing we're going to run into (I can reproduce it in my fork):

❯ gh workflow list --repo mamercad/stackstorm-exchange-ci      
Enable Pack Build and Test  active  43546553
CI - Build and Test         active  43398038
Test                        active  43324335

❯ gh workflow enable "Enable Pack Build and Test" --repo mamercad/stackstorm-exchange-ci
could not find any workflows named Enable Pack Build and Test

❯ gh workflow disable "Enable Pack Build and Test" --repo mamercad/stackstorm-exchange-ci
✓ Disabled Enable Pack Build and Test

❯ gh workflow enable "Enable Pack Build and Test" --repo mamercad/stackstorm-exchange-ci
✓ Enabled Enable Pack Build and Test

The second command there is problematic if the workflow is already enabled since it exits nonzero:

❯ gh workflow enable "Enable Pack Build and Test" --repo mamercad/stackstorm-exchange-ci; echo $?
could not find any workflows named Enable Pack Build and Test
1

@mamercad mamercad deleted the enable-inactive-pack-workflows branch December 20, 2022 11:09
@cognifloyd
Copy link
Member

Doh! Then I guess we do need an is-not-active check. Bummer.

Hmm. That check is baked into the gh client, so maybe the API won't have the same gotcha. So, we could also try using gh api .../enable instead of gh workflow enable.

I'm fine either way. Wdyt?

@mamercad
Copy link
Contributor Author

Doh! Then I guess we do need an is-not-active check. Bummer.

Hmm. That check is baked into the gh client, so maybe the API won't have the same gotcha. So, we could also try using gh api .../enable instead of gh workflow enable.

I'm fine either way. Wdyt?

Yeah, that's a good thought, I'll iterate in the PR.

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.

Echange Packs GH Action workflows disabled after 60days of inactivity

3 participants