-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
added cli options to output ACLs #581
Conversation
Requires integration testing with separate instances of headscale per test or by dynamically loading ACLs into a single running instance (issue #582 ) |
cmd/headscale/cli/acls.go
Outdated
|
||
return | ||
} | ||
policy := h.GetACLPolicy() |
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 won't do, you need to go through the protobuf API for this. Add message type and response in proto folder, generate code via make generate
then edit grpcv1.go
to add server-side part
@restanrm - as discussed, changes have been made to implement this over the grpc server. |
It behaves as expected what do you think of this @kradalby ? |
This still needs some things fixed up - still working on it |
g2g now - ready for review |
Hi, I am happy to give this a go, apologise for the delay, can you do a pass through the linters (proto, golangci and prettier) and then we can merge? And update the hash in |
Hey @samson4649, apologies for the delay. Can you check the conflicts? |
@samson4649 can you check the (minor) conflicts? I think we are pretty ready to merge. |
@samson4649 we have the linters working again, and found a couple of minor things here. There is also an issue with a dependency. Can you have a look? 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.
In addition to those comments, if you make changes to go.mod and go.sum, you need to update its checksum in flake.nix
.
@@ -30,15 +30,15 @@ require ( | |||
golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa | |||
golang.org/x/oauth2 v0.0.0-20220808172628-8227340efae7 | |||
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 | |||
google.golang.org/genproto v0.0.0-20220808204814-fd01256a5276 |
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.
Do not downgrade, please
google.golang.org/grpc v1.48.0 | ||
google.golang.org/protobuf v1.28.1 | ||
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c | ||
gopkg.in/yaml.v2 v2.4.0 | ||
gopkg.in/yaml.v3 v3.0.1 | ||
gorm.io/driver/postgres v1.3.8 | ||
gorm.io/gorm v1.23.8 | ||
inet.af/netaddr v0.0.0-20220617031823-097006376321 |
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.
Downgrade :(
@@ -70,7 +70,7 @@ require ( | |||
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect | |||
github.com/google/uuid v1.3.0 // indirect | |||
github.com/gookit/color v1.5.0 // indirect | |||
github.com/hashicorp/go-version v1.4.0 // indirect |
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.
Same
return h.aclPolicy, nil | ||
} | ||
|
||
func ACLProtoToStruct(v *v1.ACLPolicy) (*ACLPolicy, error) { |
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.
Here the linter is complaining
parameter name 'v' is too short for the scope of its usage (varnamelen)
Replace v
for policy
or something like this, for instance.
protoHosts := make(map[string]string, len(*h)) | ||
for k, v := range *h { | ||
protoHosts[k] = v.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.
} | |
} | |
protoHosts[k] = v.String() | ||
} | ||
return protoHosts | ||
} |
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.
} | |
} | |
output, | ||
) | ||
|
||
return |
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.
return |
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.
not required
Closing this for now, it will require a large rebase after our code reorg work goes in. If picked up again, please open when ready. |
Drafted to Fix #492