Skip to content

Comments

🦾 Automated workflows for config updates and config validation#154

Merged
shankari merged 5 commits intoe-mission:mainfrom
JGreenlee:config-update-and-validation-workflows
Jun 28, 2025
Merged

🦾 Automated workflows for config updates and config validation#154
shankari merged 5 commits intoe-mission:mainfrom
JGreenlee:config-update-and-validation-workflows

Conversation

@JGreenlee
Copy link
Member

add validate-configs workflow

index.d.ts is based on appConfigTypes.ts from e-mission-phone https://github.com/e-mission/e-mission-phone/blob/30e4b78ff64596b20cea811000f6d5b11a6b6cae/www/js/types/appConfigTypes.ts, moved here to be the source of truth for supported config options. Added package.json so this functions as a minimal npm package and the types can be used downstream on e-mission-phone

In a workflow, we can generate JSON schema from the typescript declarations and validate all the configs accordingly. This will help us catch mistakes in config creation or edits and will allow us to set up auto-merge for trusted PRs

Adding this validation exposed several inconsistencies in the config, which I have also fixed in this commit:
e-mission/op-admin-dashboard#167 (comment)


add config-update workflow and update_admin_access script

The config-update workflow runs one of the scripts in bin/config_update, commits and PRs the config changes if there are any, and auto-merges that PR if it passes checks.
The workflow can be triggered from op-admin-dashboard given that it has credentials to trigger workflows (which are provided through a Github app: https://github.com/settings/apps/op-config-updates)
Currently, this works to add/remove admin users from the admin_access list:
e-mission/op-admin-dashboard#167 (comment)

Tested end-to-end from admin dash:
e-mission/op-admin-dashboard#168


format all configs with 4 spaces

When the config-update workflow runs, it performs json.dump which causes whitespace changes if the config wasn't already in that format. To minimize this, and because it's a nice idea anyway, let's reformat them to the standard format. (From what I can tell, JavaScript JSON.stringify and python json.dump are pretty much aligned, so it doesn't matter which one we use)

New configs generated by issue-to-json will now use 4 spaces instead of 2


remove unused files, update README

  • update README
  • remove /docs (all current docs are either in this repo's README or in e-mission-docs)
  • remove qr_code_images

@shankari
Copy link
Contributor

@JGreenlee it is not worth changing it now, but for the record, GitHub actions don't have to be in python; they can also be in javascript; you should be able to use the typescript schema directly for validation!

@JGreenlee JGreenlee force-pushed the config-update-and-validation-workflows branch from a3f395a to 59f8609 Compare April 22, 2025 15:43
@JGreenlee JGreenlee moved this from Ready for review by Shankari to Issues being worked on in OpenPATH Tasks Overview Apr 22, 2025
@JGreenlee JGreenlee force-pushed the config-update-and-validation-workflows branch from 59f8609 to 6bafa2a Compare April 22, 2025 19:27
JGreenlee and others added 5 commits June 23, 2025 13:00
index.d.ts is based on appConfigTypes.ts from e-mission-phone https://github.com/e-mission/e-mission-phone/blob/30e4b78ff64596b20cea811000f6d5b11a6b6cae/www/js/types/appConfigTypes.ts, moved here to be the source of truth for supported config options. Added package.json so this functions as a minimal npm package and the types can be used downstream on e-mission-phone

In a workflow, we can generate JSON schema from the typescript declarations and validate all the configs accordingly. This will help us catch mistakes in config creation or edits and will allow us to set up auto-merge for trusted PRs

Adding this validation exposed several inconsistencies in the config, which I have also fixed in this commit:
e-mission/op-admin-dashboard#167 (comment)
The config-update workflow runs one of the scripts in bin/config_update, commits and PRs the config changes if there are any, and auto-merges that PR if it passes checks.
The workflow can be triggered from op-admin-dashboard given that it has credentials to trigger workflows (which are provided through a Github app: https://github.com/settings/apps/op-config-updates)
Currently, this works to add/remove admin users from the admin_access list:
e-mission/op-admin-dashboard#167 (comment)

Tested end-to-end from admin dash:
e-mission/op-admin-dashboard#168
When the config-update workflow runs, it performs json.dump which causes whitespace changes if the config wasn't already in that format. To minimize this, and because it's a nice idea anyway, let's reformat them to the standard format. (From what I can tell, JavaScript JSON.stringify and python json.dump are pretty much aligned, so it doesn't matter which one we use)

New configs generated by issue-to-json will now use 4 spaces instead of 2
- update README
- remove /docs (all current docs are either in this repo's README or in e-mission-docs)
- remove qr_code_images
We added the validate-configs workflow which runs automatically as a check on open PRs.
But we also have the automation process for new deployments, which opens PRs when a New Project Configuration issue is opened. It uses the default token so the PR authored by the Actions bot, which causes the checks to not run. https://github.com/peter-evans/create-pull-request/blob/main/docs/concepts-guidelines.md#triggering-further-workflow-runs

We already have the op-config-updates Github app (i.e. bot). If it authors the PR the checks will run; we just need to provide its credentials as repo Secrets.

While doing this I switched away from using peter-evans/create-pull-request to just using gh CLI commands because: i) it wouldn't work without granting the bot more permissions than are actually needed, ii) gh CLI is what I used in the config-update workflow, iii) I would argue the gh CLI is more idiomatic anyway because it is general knowledge not specific to GH actions

I also made the commit and PR messages more descriptive by passing in the abbreviated name of the deployment and not just the issue number, and I included the keyword "close #<issue number>" which will make the issue automatically close when the PR is merged https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests#linking-a-pull-request-to-an-issue

Testing done on my fork:
#37
This PR was generated from an New Project Configuration issue, authored by @op-config-updates, checks ran and passed
@JGreenlee JGreenlee force-pushed the config-update-and-validation-workflows branch from 6bafa2a to 10c38f6 Compare June 23, 2025 17:12
@JGreenlee JGreenlee moved this from Issues being worked on to Ready for review by Shankari in OpenPATH Tasks Overview Jun 23, 2025
Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

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

This is related to the config editing feature, and I don't want to hold up the reformat, so I am going to merge this and sort out the action launch later.

Comment on lines +43 to +45
- name: Commit and PR changes
env:
GITHUB_TOKEN: ${{ steps.generate_token.outputs.token }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for my understanding, we changed from using peter-evans/create-pull-request to generating the token using tibdex/github-app-token@v2 and then manually committing and creating the PR so that the PR would be "authorized" and would merge as soon as the tests passed, right?

@shankari shankari merged commit ddf0a93 into e-mission:main Jun 28, 2025
1 check passed
@github-project-automation github-project-automation bot moved this from Ready for review by Shankari to Tasks completed in OpenPATH Tasks Overview Jun 28, 2025
@shankari
Copy link
Contributor

Rebased and merged since apparently this repo is set to disallow merge commits.
@JGreenlee did you make that change?

@shankari
Copy link
Contributor

I merged this and then made a small change to #157 but it failed due to

Run tibdex/github-app-token@v2
  with:
    github_api_url: https://api.github.com/
    installation_retrieval_mode: repository
    installation_retrieval_payload: e-mission/nrel-openpath-deploy-configs
    revoke: true
  env:
    IssueNumber: 157
    OpenedBy: allieshepard11
    UrlAbbreviation: arlingtonebike
Error: Input required and not supplied: app_id
    at getInput (file:///home/runner/work/_actions/tibdex/github-app-token/v2/dist/main/index.js:1:3828)
    at parseOptions (file:///home/runner/work/_actions/tibdex/github-app-token/v2/dist/main/index.js:9:87889)
    at file:///home/runner/work/_actions/tibdex/github-app-token/v2/dist/main/index.js:9:87507
    at run (file:///home/runner/work/_actions/tibdex/github-app-token/v2/dist/main/index.js:9:88817)
    at file:///home/runner/work/_actions/tibdex/github-app-token/v2/dist/main/index.js:9:87480
    at __nccwpck_require__.a (file:///home/runner/work/_actions/tibdex/github-app-token/v2/dist/main/index.js:9:368729)
    at 399 (file:///home/runner/work/_actions/tibdex/github-app-token/v2/dist/main/index.js:9:87363)
    at __nccwpck_require__ (file:///home/runner/work/_actions/tibdex/github-app-token/v2/dist/main/index.js:9:367817)
    at file:///home/runner/work/_actions/tibdex/github-app-token/v2/dist/main/index.js:9:369615
    at ModuleJob.run (node:internal/modules/esm/module_job:263:25)

Also, the email authentication/admin access from https://github.com/e-mission/nrel-openpath-deploy-configs/actions/runs/15939180723 failed because of

Received response with len(response["UserPools"])=60 and next_token='dXMtd2VzdC0yX1pqM284elpMbQ=='
Received response with len(response["UserPools"])=39 & next_token=None
Traceback (most recent call last):
  File "/home/runner/work/nrel-openpath-deploy-configs/nrel-openpath-deploy-configs/email_automation/email-config.py", line 171, in <module>
    emails, map_trip_lines_enabled, columns_exclude = email_extract()
                                                      ^^^^^^^^^^^^^^^
  File "/home/runner/work/nrel-openpath-deploy-configs/nrel-openpath-deploy-configs/email_automation/email-config.py", line 105, in email_extract
    emails = [i.strip().lstrip() for i in admindash_prefs['admin_access']]
                                          ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^
KeyError: 'admin_access'

@shankari
Copy link
Contributor

To fix the tibdex/github-app-token@v2, I went in and configured the CONFIG_UPDATES_GH_APP_ID secret, but I wasn't sure how to generate a private key for it since I don't see it in the list of GitHub apps in the developer settings

In the left sidebar, click Developer settings.
In the left sidebar, click GitHub Apps.
Next to the GitHub App that you want to generate a private key for, click Edit.

I do see it in "Third Party Access"; not sure how to register it so I can also see it in the developer settings

@shankari
Copy link
Contributor

The second issue was just because there really wasn't an admin_access entry in the ca-ebike config!! I pulled out the list of currently authorized users and added them to the config in 198ad6b

After that, both validate configs and the welcome email passed!!
Screenshot 2025-06-27 at 7 50 04 PM

@shankari
Copy link
Contributor

but the issue conversion still fails when I rerun the action for #157 with the error

Run tibdex/github-app-token@v2
Error: Input required and not supplied: private_key

@shankari
Copy link
Contributor

@JGreenlee remaining tasks for this PR:

  • configure the private key
  • add a check for the admin_access to the config validation

@JGreenlee
Copy link
Member Author

In total there are 3 env vars we will need: CONFIG_UPDATES_GH_APP_ID, CONFIG_UPDATES_GH_APP_PRIVATE_KEY, and CONFIG_UPDATES_GH_APP_ID

Sent them to you privately

@JGreenlee
Copy link
Member Author

JGreenlee commented Jun 28, 2025

There are several configs (about 20 of them) that don't have "admin_access" defined, which is why it isn't currently marked as a required field

We should either update all of these or have the email-config.py script skip if admin_access is not defined instead of failing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Tasks completed

Development

Successfully merging this pull request may close these issues.

2 participants