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: load and parse account management resources #1235

Merged
merged 6 commits into from
Oct 18, 2023
Merged

Conversation

warber
Copy link
Contributor

@warber warber commented Oct 13, 2023

What this PR does / Why we need it:

This PR introduces the capability to read account management resources from files living inside of monaco project folders. The PR contains a function responsible for loading raw resources into a first in-memory representation as well as a function validating the references between users groups and policies.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

@warber warber marked this pull request as ready for review October 13, 2023 14:06
@warber warber requested a review from a team as a code owner October 13, 2023 14:06
@github-actions
Copy link

github-actions bot commented Oct 13, 2023

Unit Test Results

       2 files  ±  0     222 suites  +4   18s ⏱️ -6s
1 505 tests +13  1 505 ✔️ +13  0 💤 ±0  0 ±0 
3 010 runs  +26  3 010 ✔️ +26  0 💤 ±0  0 ±0 

Results for commit 97ba2ad. ± Comparison against base commit f3b31cf.

♻️ This comment has been updated with latest results.

@warber warber force-pushed the feat/load-acc-resources branch from 561dde1 to 8ac61c6 Compare October 16, 2023 07:39
Copy link
Contributor

@UnseenWizzard UnseenWizzard left a comment

Choose a reason for hiding this comment

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

IMO validation should be improved.

pkg/persistence/account/load.go Show resolved Hide resolved
pkg/persistence/account/validate.go Outdated Show resolved Hide resolved
pkg/persistence/account/types.go Show resolved Hide resolved
pkg/persistence/account/validate.go Show resolved Hide resolved
@warber
Copy link
Contributor Author

warber commented Oct 17, 2023

@UnseenWizzard I've improved the PR, pls take another look

// Load loads account management resources from YAML configuration files
// located within the specified root directory path. It parses the YAML files, extracts policies,
// groups, and users data, and organizes them into a AMResources struct, which is then returned.
func Load(fs afero.Fs, rootPath string) (*AMResources, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd mark this as excluded from the gocognit check.
I've quickly tried if it would actually become nicer to read if we split out the parts into loadPolicies, loadGroups, .. methods. But with error handling still needed there's not much of a difference at all, actually it becomes messier to read.

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 the "complexity" is absolutely manageable

@warber warber force-pushed the feat/load-acc-resources branch from 095e81e to 97ba2ad Compare October 18, 2023 09:32
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@warber warber merged commit 31b0c6a into main Oct 18, 2023
10 of 11 checks passed
@warber warber deleted the feat/load-acc-resources branch October 18, 2023 10:08
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.

2 participants