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

support directory walking and multiple documents per file #244

Merged
merged 2 commits into from
Apr 17, 2024

Conversation

mikemrm
Copy link
Contributor

@mikemrm mikemrm commented Apr 12, 2024

This extends the directory support to include recursive directories for further organization.

Additionally, this adds support for multiple yaml documents in a single file which allows for simple organization without cluttering folders unnecessarily.

This extends the directory support to include recursive directories for further organization.

Additionally, this adds support for multiple yaml documents in a single file which allows for simple organization without cluttering folders.

Signed-off-by: Mike Mason <[email protected]>
@mikemrm mikemrm marked this pull request as ready for review April 15, 2024 18:31
@mikemrm mikemrm requested review from a team as code owners April 15, 2024 18:31
cmd/schema.go Outdated
Comment on lines 79 to 101
err := filepath.WalkDir(cfg.SpiceDB.PolicyDir, func(path string, entry fs.DirEntry, err error) error {
if err != nil {
return err
}

for _, file := range files {
if !file.IsDir() && (strings.EqualFold(filepath.Ext(file.Name()), ".yml") || strings.EqualFold(filepath.Ext(file.Name()), ".yaml")) {
filePaths = append(filePaths, cfg.SpiceDB.PolicyDir+"/"+file.Name())
if entry.IsDir() {
return nil
}

ext := filepath.Ext(entry.Name())

if strings.EqualFold(ext, ".yml") || strings.EqualFold(ext, ".yaml") {
filePaths = append(filePaths, path)
}

return nil
})

if err != nil {
logger.Fatalw("failed to read policy files from directory", "error", err)
}

outputPolicyMermaid(filePaths, viper.GetBool("mermaid-markdown"))
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this code path ultimately leads to creating a policy anyway. Can we just use NewPolicyFromDirectory here rather than duplicate logic?

Copy link
Contributor Author

@mikemrm mikemrm Apr 16, 2024

Choose a reason for hiding this comment

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

We cannot, as NewPolicyFromDirectory returns a Policy which is a compiled policy, where as the mermaid document works off the raw PolicyDocument structure.

Once converted to a Policy we lose some helpful details for generating the mermaid document.

However we could just create NewPolicyDocumentFromDirectory which contains the logic that both NewPolicyFromDirectory and the mermaid code could use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Split this out into LoadPolicyDocumentFromDirectory which now both can use.

…w policies and mermaid to remove duplicate code

Signed-off-by: Mike Mason <[email protected]>
@mikemrm mikemrm merged commit c71c129 into infratographer:main Apr 17, 2024
4 checks passed
@mikemrm mikemrm deleted the recursive-dir branch April 17, 2024 14:07
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