-
Notifications
You must be signed in to change notification settings - Fork 38
feat: support storage and use of custom rego engine hostnames #2315
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: support storage and use of custom rego engine hostnames #2315
Conversation
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
policiesAllowedHostnames = result.GetPoliciesAllowedHostnames() | ||
|
||
signingOpts := result.GetSigningOptions() | ||
if signingOpts != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is actually not needed, right? proto getters are always safe and work for nil
receivers, so it's safe to do GetSigningOptions().GetTimestampAuthorityUrl()
even if SigningOptions is nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me double check it
ID, Name string | ||
CreatedAt *time.Time | ||
PolicyViolationBlockingStrategy string | ||
PolicyAllowedHostnames []string `json:"policyAllowedHostnames,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accidental annotation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope really, I wanted to hide this option if the default is set
type NewOrgUpdateOpts struct { | ||
BlockOnPolicyViolation *bool | ||
BlockOnPolicyViolation *bool | ||
PoliciesAllowedHostnames *[]string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a bit weird. Slices are pointers already and are nil
if no initialized. No need to create a pointer to a pointer. Just keep in mind that len([]string(nil))
is 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checking, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated the pointer situation to leverage instead the duality of empty slice vs nil slice.
} | ||
|
||
if opts.PoliciesAllowedHostnames != nil { | ||
payload.PoliciesAllowedHostnames = *opts.PoliciesAllowedHostnames |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I mean, *variable
shouldn't be needed for slices, since they are pointers anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is about making sure we can detect when we want to update the value vs empty it. In any case I've implemented the same by using slices
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
This PR allows to store, expose and use custom hostnames that can be used inside policies. If provided, they will be appended to the existing ones.
This PR includes
org describe
andatt init
response.closes #2267