Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/policy refactor (#3) (v4.0.0 compatible) #740

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

StewartW
Copy link
Contributor

@StewartW StewartW commented Jun 14, 2024

Why?

A refactor of how policies (SCP/Tagging) are managed.

Provides backwards compatibility with the current process whilst enabling an additional folder "adf-policies" that allows for SCPs, Tagging policies (and potentially more) to be defined in a central location. Allowing for policies to be-used across multiple targets. (Currently supports OU-names, but also supports extension going forward)

Issue #, if available:
What?

Description of changes:

Creates a new concept called Policy Campaigns. Which is an object used to orchestrate the updating of policies and their targets.

Policy Campaigns have a list of Policies that require creating. (A policy is classed as requiring creation when a policy with the same name does not exist). Policies that require updating (A policy is classed as requiring updating when an existing policy has a different content) and policies that require deleting (Any policy that is ADF managed but does not receive an interaction throughout the campaign is considered for deletion)

Policies themselves do similar logic for targets, and depending on the adf config, a target will either have the default SCP maintained or removed.

The overall logic is the same for the legacy policy management, the primary difference is that adf-policies is now an optional source for policies.

By submitting this pull request, I confirm that you can use, modify, copy, and
redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@javydekoning javydekoning left a comment

Choose a reason for hiding this comment

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

Thanks @StewartW , does this also address #735 , #724 ?

Makefile Outdated Show resolved Hide resolved
)
parameter_store.put_parameter(f"cross_region/{key}/{region}", value)
parameter_store.put_parameter(f"/cross_region/{key}/{region}", value)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we prefix ADF as we do for all of ADFv4 params?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, you're right. Good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually already done by the parameter_store class. (L100)

@StewartW
Copy link
Contributor Author

Thanks @StewartW , does this also address #735 , #724 ?

Yeah, it addresses that. As well as an issue raised for an AWS partner

* Starting refactor of policy application

* initial testing

* Unit tests for OrganisationPolicy class

* Linting

* wip

* Merging

* sìos leis a' Bheurla

* Resetting generate params

* Fixing spelling mistakes

* Updating documentation

* Apply suggestions from code review

Co-authored-by: Simon Kok <sbkok@users.noreply.github.com>

* Update src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organization_policy_campaign.py

Co-authored-by: Simon Kok <sbkok@users.noreply.github.com>

* Update src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/main.py

Co-authored-by: Simon Kok <sbkok@users.noreply.github.com>

* Update src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/organization_policy_campaign.py

Co-authored-by: Simon Kok <sbkok@users.noreply.github.com>

* fixing tests

* fixing linting

* linting again

* temp remove assertion

* updating logging

* running black with ll 80

* linting

* Tox no longer complaining

---------

Co-authored-by: Simon Kok <mail@simonkok.com>
Co-authored-by: Simon Kok <sbkok@users.noreply.github.com>
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.

None yet

2 participants