feat: Add config-manager push authz-policies command - #83
Conversation
d6fc273 to
5c4344d
Compare
5c4344d to
8c46fde
Compare
8c46fde to
15f3f74
Compare
a83440c to
062a70e
Compare
062a70e to
e417383
Compare
|
|
||
| const deploymentTypes = [ | ||
| CLOUD_DEPLOYMENT_TYPE_KEY, | ||
| FORGEOPS_DEPLOYMENT_TYPE_KEY, |
There was a problem hiding this comment.
No need to specify deployment types in this file, all deployments support authz policies, including in fr config manager (see https://github.com/ForgeRock/fr-config-manager/blob/e777695e00a4e92f3bc9f3961f21c8a66f9a6d3c/packages/fr-config-common/src/constants.js#L85-L139)
| const realmsDir = getFilePath('realms/'); | ||
| realmsToProcess = fs | ||
| .readdirSync(realmsDir, { withFileTypes: true }) | ||
| .filter((entry) => entry.isDirectory()) | ||
| .map((entry) => entry.name); |
There was a problem hiding this comment.
We have a problem here. When I do an export with Forgeops, it looks like this:
Basically, the root realm is problematic. It will think "authorization" is a realm, when it isn't. I think we'll need to fix the pull command to, when it exports from root realm, to save it into a directory called root. The other thing is that authz-policies are supported in the Cloud root realm as well, so this isn't deployment specific, it applies to all of them.
The root directory should be named root, and then realms like alpha and bravo can be named as they usually are. Then, when you read in the realms, just make sure that for root you are setting it to / since root is not actually the name of it, / is.
There was a problem hiding this comment.
I noticed when going through this PR again that the way that authz policies pull was originally implemented is very wrong to begin with. Currently Brycen is doing some refactoring to fix it, so I'm thinking you wait until his changes get merged in before continuing on this PR since the exports are not entirely correct.
Once that's done, here are a few other things to do in addition to my other comments that I left:
- For the test data that you are using, please use the config from https://git.trivir.com/projects/TRIVIR/repos/frodo-forgeops-test-config, as it has config for each realm. Your tests are only testing the alpha realm at the moment, but there is test config for testing the root realm as well.
- Delete the old test mocks, there are multiple mocks that are committed that need to be deleted.
- Make sure you are running
npm lint:fixafter you make any changes since there are some formatting issues
| .addOption( | ||
| new Option( | ||
| '-n, --policy-name <set-name>', | ||
| 'Policy set name. If specified, only the policy set with the specified name is imported.' | ||
| ) | ||
| ) |
There was a problem hiding this comment.
I just checked against fr-config-manager, and they don't actually have a -n flag for their fr-config-push command, so I think we remove it from our implementation as well.
| const getTokensIsSuccessful = await getTokens(); | ||
| if (!getTokensIsSuccessful) process.exit(1); | ||
| printMessage( | ||
| `Importing organization privileges config for authz policies` |
There was a problem hiding this comment.
This message is not importing organization privileges, it should just say something like Importing authorization policies...
| !configFileData[state.getRealm()] || | ||
| !configFileData[state.getRealm()].includes(ps.name) |
There was a problem hiding this comment.
I would replace state.getRealm() in both these lines with a variable, which is set before as const realmDir = state.getRealm() === '/' ? 'root' : state.getRealm();, this way the configuration file can specify "root" as a realm instead of calling it "/", I think it should match the directory names where it's saved. You can set the variable before the if (configFile) since we'll be using it again when we save the config
|
|
||
| /** | ||
| * Import authz policy sets | ||
| * @param name optional name to import |
| * @returns {Promise<boolean>} true if all imports were successful | ||
| */ | ||
| export async function configManagerImportAuthzPolicies( | ||
| name: string |
| /* | ||
| // ForgeOps | ||
| FRODO_MOCK=record FRODO_NO_CACHE=1 FRODO_HOST=https://nightly.gcp.forgeops.com/am frodo config-manager push authz-policies -D test/e2e/exports/fr-config-manager/forgeops -m forgeops | ||
| FRODO_MOCK=record FRODO_NO_CACHE=1 FRODO_HOST=https://nightly.gcp.forgeops.com/am frodo config-manager push authz-policies -n test_id -D test/e2e/exports/fr-config-manager/forgeops -m forgeops |
2 participants
No description provided.