feat: split up team policies to single file (#2057)

Author: CasLubbers

src/cmd/commit.ts

@@ -70,8 +70,9 @@ const commitAndPush = async (values: Record<string, any>, branch: string): Promi
         }
         await $`git push -u origin ${branch}`
       } catch (e) {
-        d.warn(`The values repository is not yet reachable.`)
-        throw new Error('Could not commit and push. Retrying...')
+        const errorMsg = 'Could not pull and push. Retrying...'
+        d.error(errorMsg)
+        throw new Error(errorMsg)
       }
     },
     {

src/cmd/migrate.test.ts

@@ -1,5 +1,8 @@
-import { applyChanges, Changes, filterChanges, getBuildName } from 'src/cmd/migrate'
+import { applyChanges, Changes, filterChanges, getBuildName, policiesMigration } from 'src/cmd/migrate'
 import stubs from 'src/test-stubs'
+import { globSync } from 'glob'
+import { getFileMap } from '../common/repo'
+import { env } from '../common/envalid'
 
 jest.mock('uuid', () => ({
   v4: jest.fn(() => 'my-fixed-uuid'),
@@ -567,3 +570,57 @@ describe('Build image name migration', () => {
     expect(deps.writeValues).toBeCalledWith(expectedValues, true)
   }, 20000)
 })
+
+jest.mock('glob')
+describe('Policies migration', () => {
+  const mockFilePaths = ['/path/to/env/teams/admin/policies.yaml', '/path/to/env/teams/alpha/policies.yaml']
+
+  const policiesFileMap = getFileMap('AplTeamPolicy', env.ENV_DIR)
+  const mockYamlContent = {
+    '/path/to/env/teams/admin/policies.yaml': {
+      metadata: { name: 'admin' },
+      spec: { ruleA: { action: 'Audit' } },
+    },
+    '/path/to/env/teams/alpha/policies.yaml': {
+      metadata: { name: 'alpha' },
+      spec: { ruleB: { action: 'Enforce' } },
+    },
+  }
+
+  const loadYaml = jest.fn((filePath: string) => mockYamlContent[filePath])
+  const saveResourceGroupToFiles = jest.fn()
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+  })
+
+  it('should load and convert policies.yaml files into teamConfig and save them', async () => {
+    ;(globSync as jest.Mock).mockReturnValue(mockFilePaths)
+
+    await policiesMigration({ loadYaml, saveResourceGroupToFiles })
+
+    expect(loadYaml).toHaveBeenCalledTimes(2)
+    expect(loadYaml).toHaveBeenCalledWith('/path/to/env/teams/admin/policies.yaml')
+    expect(loadYaml).toHaveBeenCalledWith('/path/to/env/teams/alpha/policies.yaml')
+
+    expect(saveResourceGroupToFiles).toHaveBeenCalledWith(
+      policiesFileMap,
+      {
+        teamConfig: {
+          admin: { policies: { ruleA: { action: 'Audit' } } },
+          alpha: { policies: { ruleB: { action: 'Enforce' } } },
+        },
+      },
+      {},
+    )
+  })
+
+  it('should not migrate if filepaths are empty', async () => {
+    ;(globSync as jest.Mock).mockReturnValue([])
+
+    await policiesMigration({ loadYaml, saveResourceGroupToFiles })
+
+    expect(loadYaml).toHaveBeenCalledTimes(0)
+    expect(saveResourceGroupToFiles).toHaveBeenCalledTimes(0)
+  })
+})

src/cmd/migrate.ts

@@ -5,15 +5,15 @@ import { randomUUID } from 'crypto'
 import { diff } from 'deep-diff'
 import { copy, createFileSync, move, pathExists, renameSync, rm } from 'fs-extra'
 import { mkdir, readFile, writeFile } from 'fs/promises'
-import { glob } from 'glob'
+import { glob, globSync } from 'glob'
 import { cloneDeep, each, get, isObject, isUndefined, mapKeys, mapValues, omit, pick, pull, set, unset } from 'lodash'
 import { basename, dirname, join } from 'path'
 import { prepareEnvironment } from 'src/common/cli'
 import { decrypt, encrypt } from 'src/common/crypt'
 import { terminal } from 'src/common/debug'
 import { env } from 'src/common/envalid'
 import { hf, hfValues } from 'src/common/hf'
-import { getFileMap, getTeamNames, saveValues } from 'src/common/repo'
+import { getFileMap, getTeamNames, saveResourceGroupToFiles, saveValues } from 'src/common/repo'
 import { getFilename, getSchemaSecretsPaths, gucci, loadYaml, rootDir } from 'src/common/utils'
 import { writeValues, writeValuesToFile } from 'src/common/values'
 import { BasicArguments, getParsedArgs, setParsedArgs } from 'src/common/yargs'
@@ -53,6 +53,7 @@ interface Change {
   networkPoliciesMigration?: boolean
   teamResourceQuotaMigration?: boolean
   buildImageNameMigration?: boolean
+  policiesMigration?: boolean
 }
 
 export type Changes = Array<Change>
@@ -310,6 +311,32 @@ export const getBuildName = (name: string, tag: string): string => {
     .replace(/^-|-$/g, '') // Remove leading or trailing hyphens
 }
 
+export async function policiesMigration(deps = { loadYaml, saveResourceGroupToFiles }) {
+  const filePaths = globSync(`${env.ENV_DIR}/env/teams/*/policies.yaml`, {
+    nodir: true, // Exclude directories
+    dot: false,
+  }).sort()
+  if (filePaths.length === 0) {
+    console.info('No policies.yaml files found in env/teams/*/policies.yaml. Skipping migration')
+    return
+  }
+  const inValues = filePaths.map(async (filePath) => {
+    const yaml = await deps.loadYaml(filePath)
+    return {
+      [yaml!.metadata.name]: {
+        policies: yaml!.spec,
+      },
+    }
+  })
+  const teamArray = await Promise.all(inValues)
+  const teamConfig = teamArray.reduce((acc, team) => {
+    return { ...acc, ...team }
+  }, {})
+
+  const policiesFileMap = getFileMap('AplTeamPolicy', env.ENV_DIR)
+  await deps.saveResourceGroupToFiles(policiesFileMap, { teamConfig }, {})
+}
+
 const buildImageNameMigration = async (values: Record<string, any>): Promise<void> => {
   const teams: Array<string> = Object.keys(values?.teamConfig as Record<string, any>)
   type Build = {
@@ -412,6 +439,7 @@ export const applyChanges = async (
     if (c.networkPoliciesMigration) await networkPoliciesMigration(values)
     if (c.teamResourceQuotaMigration) teamResourceQuotaMigration(values)
     if (c.buildImageNameMigration) await buildImageNameMigration(values)
+    if (c.policiesMigration) await policiesMigration()
 
     Object.assign(values.versions, { specVersion: c.version })
   }

src/common/repo.test.ts

@@ -104,11 +104,11 @@ describe('File map constraints', () => {
     const maps = getFileMaps('/tmp')
     maps.forEach((item) => {
       expect(item.jsonPathExpression.startsWith('$.')).toBe(true)
-      if (item.processAs === 'arrayItem') {
+      if (item.processAs === 'arrayItem' || item.kind === 'AplTeamPolicy') {
         expect(item.jsonPathExpression.endsWith('[*]')).toBe(true)
       }
       if (item.processAs === 'mapItem') {
-        expect(item.jsonPathExpression.endsWith('[*]')).toBe(false)
+        expect(item.jsonPathExpression.endsWith('[*]') && item.kind !== 'AplTeamPolicy').toBe(false)
       }
       if (item.resourceGroup === 'team') {
         expect(item.jsonPathExpression.startsWith('$.teamConfig.*.')).toBe(true)

src/common/repo.ts

@@ -81,7 +81,8 @@ export function getResourceName(fileMap: FileMap, jsonPath: jsonpath.PathCompone
     return resourceName
   }
 
-  if (fileMap.resourceGroup === 'team') {
+  // Custom workaround for teamPolicy because it is a mapItem
+  if (fileMap.resourceGroup === 'team' && fileMap.kind !== 'AplTeamPolicy') {
     resourceName = getTeamNameFromJsonPath(jsonPath)
     return resourceName
   } else {
@@ -369,11 +370,11 @@ export function getFileMaps(envDir: string): Array<FileMap> {
     {
       kind: 'AplTeamPolicy',
       envDir,
-      jsonPathExpression: '$.teamConfig.*.policies',
-      pathGlob: `${envDir}/env/teams/*/policies.yaml`,
+      jsonPathExpression: '$.teamConfig.*.policies[*]',
+      pathGlob: `${envDir}/env/teams/*/policies/*.yaml`,
       processAs: 'mapItem',
       resourceGroup: 'team',
-      resourceDir: '.',
+      resourceDir: 'policies',
       loadToSpec: true,
     },
   ]
@@ -595,6 +596,13 @@ export async function loadFileToSpec(
     if (fileMap.processAs === 'arrayItem') {
       const ref: Record<string, any>[] = get(spec, jsonPath)
       ref.push(data?.spec)
+    } else if (fileMap.kind === 'AplTeamPolicy') {
+      const ref: Record<string, any> = get(spec, jsonPath)
+      const policy = {
+        [data?.metadata?.name]: data?.spec,
+      }
+      const newRef = merge(cloneDeep(ref), policy)
+      set(spec, jsonPath, newRef)
     } else {
       const ref: Record<string, any> = get(spec, jsonPath)
       // Decrypted secrets may need to be merged with plain text specs

tests/fixtures/env/teams/admin/policies.yaml

@@ -0,0 +1,8 @@
+kind: AplTeamPolicy
+metadata:
+    name: allowed-image-repositories
+    labels:
+        apl.io/teamId: admin
+spec:
+    action: Audit
+    severity: medium

tests/fixtures/env/teams/admin/policies/allowed-image-repositories.yaml

@@ -0,0 +1,8 @@
+kind: AplTeamPolicy
+metadata:
+    name: disallow-capabilities-strict
+    labels:
+        apl.io/teamId: admin
+spec:
+    action: Audit
+    severity: medium

tests/fixtures/env/teams/admin/policies/disallow-capabilities-strict.yaml

@@ -0,0 +1,23 @@
+kind: AplTeamPolicy
+metadata:
+    name: disallow-capabilities
+    labels:
+        apl.io/teamId: admin
+spec:
+    action: Audit
+    customValues:
+        - AUDIT_WRITE
+        - CHOWN
+        - DAC_OVERRIDE
+        - FOWNER
+        - FSETID
+        - KILL
+        - MKNOD
+        - NET_BIND_SERVICE
+        - SETFCAP
+        - SETGID
+        - SETPCAP
+        - SETUID
+        - SYS_CHROOT
+        - "''"
+    severity: medium

tests/fixtures/env/teams/admin/policies/disallow-capabilities.yaml

@@ -0,0 +1,8 @@
+kind: AplTeamPolicy
+metadata:
+    name: disallow-host-namespaces
+    labels:
+        apl.io/teamId: admin
+spec:
+    action: Audit
+    severity: medium

tests/fixtures/env/teams/admin/policies/disallow-host-namespaces.yaml

@@ -0,0 +1,8 @@
+kind: AplTeamPolicy
+metadata:
+    name: disallow-host-path
+    labels:
+        apl.io/teamId: admin
+spec:
+    action: Audit
+    severity: medium

tests/fixtures/env/teams/admin/policies/disallow-host-path.yaml

@@ -0,0 +1,8 @@
+kind: AplTeamPolicy
+metadata:
+    name: disallow-host-ports
+    labels:
+        apl.io/teamId: admin
+spec:
+    action: Audit
+    severity: medium

tests/fixtures/env/teams/admin/policies/disallow-host-ports.yaml

@@ -0,0 +1,8 @@
+kind: AplTeamPolicy
+metadata:
+    name: disallow-host-process
+    labels:
+        apl.io/teamId: admin
+spec:
+    action: Audit
+    severity: medium

tests/fixtures/env/teams/admin/policies/disallow-host-process.yaml

@@ -0,0 +1,8 @@
+kind: AplTeamPolicy
+metadata:
+    name: disallow-latest-tag
+    labels:
+        apl.io/teamId: admin
+spec:
+    action: Audit
+    severity: medium