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

allow custom label openpolicyagent.org/policy=rego #89

Merged
merged 2 commits into from
Mar 29, 2021

Conversation

kirk-patton
Copy link

--policy-label=my.org/policy
--policy-value=mutating

I would like two run separate OPA servers using the same policy namespace. Currently openpolicyagent.org/policy=rego" is hard coded.

…/custom label.

Signed-off-by: Kirk Patton <kpatton@verizonmedia.com>
Comment on lines 78 to 79
rootCmd.Flags().StringVarP(&params.policyLabel, "policy-label", "", "", "replace label openpolicyagent.org/policy")
rootCmd.Flags().StringVarP(&params.policyValue, "policy-value", "", "", "replace value rego")
Copy link
Member

Choose a reason for hiding this comment

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

How about we make the default values "openpolicyagent.org/policy" and "rego". Then we just have to plumb the values into the configmap watcher.

return "", err
}

policyLabelKey = key
Copy link
Member

Choose a reason for hiding this comment

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

Hrmmm. Those values were defined as globals because they weren't changing (i.e., they ought to have been consts). I would rather make the main() function pass the values into the watcher as parameters rather than relying on mutable global state.

Signed-off-by: Kirk Patton <kpatton@verizonmedia.com>
@kirk-patton
Copy link
Author

@tsandall does the update address your noted feedback?

Copy link
Member

@tsandall tsandall left a comment

Choose a reason for hiding this comment

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

Yep! Thanks @kirk-patton !

@tsandall tsandall merged commit f2a7d22 into open-policy-agent:master Mar 29, 2021
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