chore: unify shared actions signatures (#169)

* chore: unify shared actions signatures

* docs: udpated changelog

* chore: apply the suggestion from copilot

Author: galargh

CHANGELOG.md

@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - new args for repositories and branch protection rules
 
 ### Changed
+- Updated the signatures of all the shared actions; now the runAction function will persist the changes to disk while action functions will operate on the in-memory state
 - Synchronization script: to use GitHub API directly instead of relying on TF GH Provider's Data Sources
 - Configuration: replaced multiple JSONs with a single, unified YAML
 - Synchronization script: rewrote the script in JS

docs/HOWTOS.md

@@ -118,7 +118,7 @@ I want to ensure that all the public repositories in my organization have their
 To do that, I ensure the following content is present in `scripts/src/actions/fix-yaml-config.ts`:
 ```ts
 import 'reflect-metadata'
-import { protectDefaultBranches } from './shared/protect-default-branches'
+import { runProtectDefaultBranches } from './shared/protect-default-branches'
 
-protectDefaultBranches()
+runProtectDefaultBranches()
 ```

scripts/__tests__/__resources__/github/default.yml

@@ -160,27 +160,7 @@ repositories:
     visibility: public
     vulnerability_alerts: false
   rust-sccache-action:
-    advanced_security: false
-    allow_auto_merge: false
-    allow_merge_commit: true
-    allow_rebase_merge: true
-    allow_squash_merge: true
-    archived: false
-    auto_init: false
-    default_branch: main
-    delete_branch_on_merge: false
-    has_downloads: true
-    has_issues: true
-    has_projects: true
-    has_wiki: true
-    is_template: false
-    secret_scanning_push_protection: false
-    secret_scanning: false
-    teams:
-      maintain:
-        - ipdx
-    visibility: public
-    vulnerability_alerts: true
+    archived: true
   tf-aws-gh-runner:
     advanced_security: false
     allow_auto_merge: false

scripts/__tests__/__resources__/terraform/terraform.tfstate

@@ -618,7 +618,7 @@
             "allow_merge_commit": true,
             "allow_rebase_merge": true,
             "allow_squash_merge": true,
-            "archived": false,
+            "archived": true,
             "branches": [
               {
                 "name": "arm64",
@@ -1644,7 +1644,7 @@
             "allow_rebase_merge": true,
             "allow_squash_merge": true,
             "archive_on_destroy": null,
-            "archived": false,
+            "archived": true,
             "auto_init": false,
             "branches": [
               {

scripts/__tests__/sync.test.ts

@@ -9,6 +9,7 @@ import {GitHub} from '../src/github'
 import env from '../src/env'
 import {Resource} from '../src/resources/resource'
 import {RepositoryFile} from '../src/resources/repository-file'
+import {toggleArchivedRepos} from '../src/actions/shared/toggle-archived-repos'
 
 test('sync', async () => {
   const yamlConfig = new config.Config('{}')
@@ -17,6 +18,7 @@ test('sync', async () => {
   const expectedYamlConfig = config.Config.FromPath()
 
   await sync(tfConfig, yamlConfig)
+  await toggleArchivedRepos(tfConfig, yamlConfig)
 
   yamlConfig.format()
 

scripts/__tests__/terraform/state.test.ts

@@ -12,12 +12,12 @@ test('can retrieve resources from tf state', async () => {
   for (const resourceClass of ResourceConstructors) {
     const classResources = config.getResources(resourceClass)
     expect(classResources).toHaveLength(
-      global.ResourceCounts[resourceClass.name]
+      global.StateResourceCounts[resourceClass.name]
     )
     resources.push(...classResources)
   }
 
-  expect(resources).toHaveLength(global.ResourcesCount)
+  expect(resources).toHaveLength(global.StateResourcesCount)
 })
 
 test('can ignore resource types', async () => {

scripts/__tests__/yaml/config.test.ts

@@ -13,7 +13,7 @@ import {randomUUID} from 'crypto'
 import {Team, Privacy as TeamPrivacy} from '../../src/resources/team'
 import {RepositoryBranchProtectionRule} from '../../src/resources/repository-branch-protection-rule'
 import {toggleArchivedRepos} from '../../src/actions/shared/toggle-archived-repos'
-import * as state from '../../src/terraform/state'
+import {State} from '../../src/terraform/state'
 
 test('can retrieve resources from YAML schema', async () => {
   const config = Config.FromPath()
@@ -23,13 +23,13 @@ test('can retrieve resources from YAML schema', async () => {
   for (const resourceClass of ResourceConstructors) {
     const classResources = config.getResources(resourceClass)
     expect(classResources).toHaveLength(
-      global.ResourceCounts[resourceClass.name]
+      global.ConfigResourceCounts[resourceClass.name]
     )
     resources.push(...classResources)
   }
 
   expect(resources).toHaveLength(
-    Object.values(global.ResourceCounts).reduce(
+    Object.values(global.ConfigResourceCounts).reduce(
       (a: number, b: number) => a + b,
       0
     )
@@ -58,7 +58,7 @@ test('can remove members', async () => {
   }
 
   expect(config.getAllResources()).toHaveLength(
-    global.ResourcesCount - global.ResourceCounts[Member.name]
+    global.ConfigResourcesCount - global.ConfigResourceCounts[Member.name]
   )
 })
 
@@ -76,8 +76,8 @@ test('can remove repositories, including their sub-resources', async () => {
   }
 
   const count =
-    global.ResourcesCount -
-    Object.entries(global.ResourceCounts).reduce(
+    global.ConfigResourcesCount -
+    Object.entries(global.ConfigResourceCounts).reduce(
       (a: number, [key, value]) =>
         key.startsWith(Repository.name) ? a + value : a,
       0
@@ -98,12 +98,12 @@ test('can add members', async () => {
     config.addResource(member)
     expect(config.someResource(member)).toBeTruthy()
     expect(config.getResources(Member)).toHaveLength(
-      global.ResourceCounts[Member.name] + index + 1
+      global.ConfigResourceCounts[Member.name] + index + 1
     )
   }
 
   expect(config.getAllResources()).toHaveLength(
-    global.ResourcesCount + members.length
+    global.ConfigResourcesCount + members.length
   )
 })
 
@@ -124,15 +124,15 @@ test('can add files, including their parent resources', async () => {
   }
 
   const count =
-    global.ResourcesCount +
+    global.ConfigResourcesCount +
     files.filter(f => !config.someResource(f)).length +
     repositories.filter(r => !config.someResource(r)).length
 
   for (const [index, file] of files.entries()) {
     config.addResource(file)
     expect(config.someResource(file)).toBeTruthy()
     expect(config.getResources(RepositoryFile)).toHaveLength(
-      global.ResourceCounts[RepositoryFile.name] + index + 1
+      global.ConfigResourceCounts[RepositoryFile.name] + index + 1
     )
   }
 
@@ -365,7 +365,7 @@ repositories:
 
 test('can add and remove resources through sync', async () => {
   const config = new Config('{}')
-  let desiredResources: Resource[] = []
+  const desiredResources: Resource[] = []
   let resources = config.getAllResources()
 
   config.sync(desiredResources)
@@ -389,41 +389,37 @@ test('can add and remove resources through sync', async () => {
 })
 
 test('clears and re-adds repository fields when archiving/unarchiving', async () => {
-  let config = Config.FromPath()
-
-  const unarchivedRepo = config.getResources(Repository).find(r => !r.archived)!
-
-  expect(unarchivedRepo.visibility).toBeDefined()
-
-  unarchivedRepo.archived = true
-  config.addResource(unarchivedRepo)
-
-  await toggleArchivedRepos(config)
+  const config = Config.FromPath()
+  const state = await State.New()
 
-  const archivedRepo = config
+  const archivedRepository = config
+    .getResources(Repository)
+    .find(r => r.archived)!
+  const unarchivedRepository = config
     .getResources(Repository)
-    .find(r => r.getStateAddress() == unarchivedRepo.getStateAddress())!
+    .find(r => !r.archived)!
 
-  expect(archivedRepo.archived).toBe(true)
-  expect(archivedRepo.visibility).not.toBeDefined()
+  expect(archivedRepository.archived).toBe(true)
+  expect(archivedRepository.visibility).not.toBeDefined()
 
-  archivedRepo.archived = false
-  config.addResource(archivedRepo)
+  expect(unarchivedRepository.archived).toBe(false)
+  expect(unarchivedRepository.visibility).toBeDefined()
 
-  const newState = JSON.parse(await state.loadState())
-  newState.values.root_module.resources.find(
-    (r: any) => r.address == unarchivedRepo.getStateAddress()
-  )!.values.archived = true
+  archivedRepository.archived = false
+  unarchivedRepository.archived = true
 
-  const loadStateMock = jest.spyOn(state, 'loadState')
-  loadStateMock.mockImplementation(async () => JSON.stringify(newState))
+  config.addResource(archivedRepository)
+  config.addResource(unarchivedRepository)
 
-  await toggleArchivedRepos(config)
+  await toggleArchivedRepos(state, config)
 
-  const toggledRepo = config
-    .getResources(Repository)
-    .find(r => r.getStateAddress() == unarchivedRepo.getStateAddress())!
+  const previouslyArchivedRepository = config.findResource(archivedRepository)!
+  const previouslyUnarchivedRepository =
+    config.findResource(unarchivedRepository)!
+
+  expect(previouslyArchivedRepository.archived).toBe(false)
+  expect(previouslyArchivedRepository.visibility).toBeDefined()
 
-  expect(toggledRepo.archived).toBe(false)
-  expect(toggledRepo.visibility).toBeDefined()
+  expect(previouslyUnarchivedRepository.archived).toBe(true)
+  expect(previouslyUnarchivedRepository.visibility).not.toBeDefined()
 })

scripts/jest.d.ts

@@ -1,6 +1,8 @@
 declare global {
-  var ResourceCounts: Record<string, number>
-  var ResourcesCount: number
+  var StateResourceCounts: Record<string, number>
+  var StateResourcesCount: number
+  var ConfigResourceCounts: Record<string, number>
+  var ConfigResourcesCount: number
 }
 
 export {}

scripts/jest.setup.ts

@@ -58,7 +58,22 @@ GitHub.github = {
   }
 } as GitHub
 
-global.ResourceCounts = {
+global.ConfigResourceCounts = {
+  [Member.name]: 2,
+  [Repository.name]: 7,
+  [Team.name]: 2,
+  [RepositoryCollaborator.name]: 1,
+  [RepositoryBranchProtectionRule.name]: 1,
+  [RepositoryTeam.name]: 6,
+  [TeamMember.name]: 2,
+  [RepositoryFile.name]: 1,
+  [RepositoryLabel.name]: 3
+}
+global.ConfigResourcesCount = Object.values(global.ConfigResourceCounts).reduce(
+  (a, b) => a + b,
+  0
+)
+global.StateResourceCounts = {
   [Member.name]: 2,
   [Repository.name]: 7,
   [Team.name]: 2,
@@ -69,7 +84,7 @@ global.ResourceCounts = {
   [RepositoryFile.name]: 1,
   [RepositoryLabel.name]: 3
 }
-global.ResourcesCount = Object.values(global.ResourceCounts).reduce(
+global.StateResourcesCount = Object.values(global.StateResourceCounts).reduce(
   (a, b) => a + b,
   0
 )

scripts/src/actions/fix-yaml-config.ts

@@ -1,17 +1,14 @@
 import 'reflect-metadata'
 
-import {Config} from '../yaml/config'
-import {toggleArchivedRepos} from './shared/toggle-archived-repos'
-import {describeAccessChanges} from './shared/describe-access-changes'
+import {runToggleArchivedRepos} from './shared/toggle-archived-repos'
+import {runDescribeAccessChanges} from './shared/describe-access-changes'
 
 import * as core from '@actions/core'
 
 async function run(): Promise<void> {
-  const config = Config.FromPath()
-  await toggleArchivedRepos(config)
-  config.save()
+  await runToggleArchivedRepos()
 
-  const accessChangesDescription = await describeAccessChanges()
+  const accessChangesDescription = await runDescribeAccessChanges()
 
   core.setOutput(
     'comment',