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.

Sign up for GitHub

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

api: Add parsed rules to policy response #6017

Merged
merged 14 commits into from
Nov 20, 2019
Merged

api: Add parsed rules to policy response #6017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0fae6f1
Add parsed rules to policy response
backspace Jul 25, 2019
069f595
Merge branch 'master' into f-policy-json
backspace Aug 29, 2019
837d484
Add standard error-handling for parse failure
backspace Aug 29, 2019
0d4275b
Add changelog entry
backspace Aug 29, 2019
ffb10e6
Change parsing error to set rules to nil
backspace Aug 29, 2019
3206229
Change test to use valid HCL for rules
backspace Aug 29, 2019
5356a78
Merge branch 'master' into f-policy-json
backspace Aug 29, 2019
0f860f4
Merge branch 'master' into f-policy-json
backspace Sep 3, 2019
1eb3cf8
Merge branch 'master' into f-policy-json
backspace Nov 20, 2019
7c7637c
Remove merge error
backspace Nov 20, 2019
241ed98
Remove extraneous whitespace
backspace Nov 20, 2019
e5a2d13
Remove extraneous else block
backspace Nov 20, 2019
4f1dbc3
Add explanatory comment
backspace Nov 20, 2019
8b18396
Add gofmt changes
backspace Nov 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ FEATURES:

IMPROVEMENTS:
* api: Add `StartedAt` field to `Node.DrainStrategy` [[GH-6698](https://github.com/hashicorp/nomad/issues/6698)]
* api: Added JSON representation of rules to policy endpoint response [[GH-6017](https://github.com/hashicorp/nomad/pull/6017)]
* build: Updated to Go 1.12.13 [[GH-6606](https://github.com/hashicorp/nomad/issues/6606)]
* core: Add support for running under Windows Service Manager [[GH-6220](https://github.com/hashicorp/nomad/issues/6220)]
* cli: Show full ID in node and alloc individual status views [[GH-6425](https://github.com/hashicorp/nomad/issues/6425)]
Expand Down
3 changes: 1 addition & 2 deletions command/acl_policy_info_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"strings"
"testing"

"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/command/agent"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
Expand All @@ -31,7 +30,7 @@ func TestACLPolicyInfoCommand(t *testing.T) {
// Create a test ACLPolicy
policy := &structs.ACLPolicy{
Name: "testPolicy",
Rules: acl.PolicyWrite,
Rules: "node { policy = \"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.

When I changed the parse error-handling to return err as @schmichael suggested, it caused a test failure because write, as stored here, isn’t valid HCL. Since the Rules aren’t actually being used anywhere in the test, I changed it to store valid HCL instead.

}
policy.SetHash()
assert.Nil(state.UpsertACLPolicies(1000, []*structs.ACLPolicy{policy}))
Expand Down
7 changes: 7 additions & 0 deletions nomad/acl_endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
metrics "github.com/armon/go-metrics"
log "github.com/hashicorp/go-hclog"
memdb "github.com/hashicorp/go-memdb"
policy "github.com/hashicorp/nomad/acl"
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 recommend not shadowing the name here, when reading the code it can make interpretation difficult when there are multiple names for one package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my previous attempt at this had it called somethingacl because whenever I didn’t have an override, this whole line just got deleted! I’m guessing because acl is already defined in the document…????

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah - I guess so :( - That's unfortunate but makes sense 👍

"github.com/hashicorp/nomad/helper/uuid"
"github.com/hashicorp/nomad/nomad/state"
"github.com/hashicorp/nomad/nomad/structs"
Expand Down Expand Up @@ -263,6 +264,12 @@ func (a *ACL) GetPolicy(args *structs.ACLPolicySpecificRequest, reply *structs.S
reply.Policy = out
if out != nil {
reply.Index = out.ModifyIndex
rules, err := policy.Parse(out.Rules)

backspace marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
reply.Policy.RulesJSON = rules
} else {
// Use the last index that affected the policy table
index, err := state.Index("acl_policy")
Expand Down
7 changes: 4 additions & 3 deletions nomad/structs/structs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9062,9 +9062,10 @@ func IsServerSide(e error) bool {

// ACLPolicy is used to represent an ACL policy
type ACLPolicy struct {
Name string // Unique name
Description string // Human readable
Rules string // HCL or JSON format
Name string // Unique name
Description string // Human readable
Rules string // HCL or JSON format
RulesJSON *acl.Policy // Generated from Rules on read
Hash []byte
CreateIndex uint64
ModifyIndex uint64
Expand Down