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

Service Discovery ACLs #1024

Merged
merged 17 commits into from
Jun 12, 2015
Merged

Service Discovery ACLs #1024

merged 17 commits into from
Jun 12, 2015

Conversation

ryanuber
Copy link
Member

Implements a filter on top of the state store to allow restricting read access to services. This allows issuing tokens which can only see services they have explicit permissions to read. The documentation is purposely left kind of half-complete so that we can polish it off after the DNS side of this is more complete, but we can tackle that in a separate PR.

@@ -126,7 +126,7 @@ func (c *Catalog) ListNodes(args *structs.DCSpecificRequest, reply *structs.Inde
state.QueryTables("Nodes"),
func() error {
reply.Index, reply.Nodes = state.Nodes()
return nil
return c.srv.filterACL(args.Token, reply)
Copy link
Member

Choose a reason for hiding this comment

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

Do nodes need to be filtered?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh probably not, haha good catch.

@armon
Copy link
Member

armon commented Jun 11, 2015

Minor comments, but LGTM! 💪 👍


case *structs.IndexedNodeDump:
filt.filterNodeDump(&v.Dump)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like the kind of thing that should freak out if a new type is encountered (we could add case clauses for ones that we explicitly don't care to filter).

Copy link
Member

Choose a reason for hiding this comment

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

Ah yeah, like a default: panic

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems reasonable, will add that in!

ryanuber added a commit that referenced this pull request Jun 12, 2015
@ryanuber ryanuber merged commit d5dbcd7 into master Jun 12, 2015
@ryanuber ryanuber deleted the f-discovery-acl branch June 12, 2015 00:49
duckhan pushed a commit to duckhan/consul that referenced this pull request Oct 24, 2021
… is offline (hashicorp#1024)

* Fail scheduling all pods that are not part of consul when the webhook is unhealthy.

Co-authored-by: Iryna Shustava <[email protected]>
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.

3 participants