From 94dbaa6822a8225183a54fd8dcb4cdeb424e9df2 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 4 Nov 2021 22:16:56 +0000 Subject: [PATCH] Clean up the return of "pointer list" This commit is getting rid of a bunch of returned list pointers. --- acls.go | 52 +++++++++---------- acls_test.go | 49 +++++++++--------- oidc_test.go | 2 +- sharing_test.go | 130 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 164 insertions(+), 69 deletions(-) diff --git a/acls.go b/acls.go index fea72a7f..4d4d45b3 100644 --- a/acls.go +++ b/acls.go @@ -15,13 +15,15 @@ import ( "tailscale.com/tailcfg" ) -const errorEmptyPolicy = Error("empty policy") -const errorInvalidAction = Error("invalid action") -const errorInvalidUserSection = Error("invalid user section") -const errorInvalidGroup = Error("invalid group") -const errorInvalidTag = Error("invalid tag") -const errorInvalidNamespace = Error("invalid namespace") -const errorInvalidPortFormat = Error("invalid port format") +const ( + errorEmptyPolicy = Error("empty policy") + errorInvalidAction = Error("invalid action") + errorInvalidUserSection = Error("invalid user section") + errorInvalidGroup = Error("invalid group") + errorInvalidTag = Error("invalid tag") + errorInvalidNamespace = Error("invalid namespace") + errorInvalidPortFormat = Error("invalid port format") +) // LoadACLPolicy loads the ACL policy from the specify path, and generates the ACL rules func (h *Headscale) LoadACLPolicy(path string) error { @@ -53,7 +55,7 @@ func (h *Headscale) LoadACLPolicy(path string) error { return nil } -func (h *Headscale) generateACLRules() (*[]tailcfg.FilterRule, error) { +func (h *Headscale) generateACLRules() ([]tailcfg.FilterRule, error) { rules := []tailcfg.FilterRule{} for i, a := range h.aclPolicy.ACLs { @@ -71,7 +73,7 @@ func (h *Headscale) generateACLRules() (*[]tailcfg.FilterRule, error) { Msgf("Error parsing ACL %d, User %d", i, j) return nil, err } - srcIPs = append(srcIPs, *srcs...) + srcIPs = append(srcIPs, srcs...) } r.SrcIPs = srcIPs @@ -83,7 +85,7 @@ func (h *Headscale) generateACLRules() (*[]tailcfg.FilterRule, error) { Msgf("Error parsing ACL %d, Port %d", i, j) return nil, err } - destPorts = append(destPorts, *dests...) + destPorts = append(destPorts, dests...) } rules = append(rules, tailcfg.FilterRule{ @@ -92,14 +94,14 @@ func (h *Headscale) generateACLRules() (*[]tailcfg.FilterRule, error) { }) } - return &rules, nil + return rules, nil } -func (h *Headscale) generateACLPolicySrcIP(u string) (*[]string, error) { +func (h *Headscale) generateACLPolicySrcIP(u string) ([]string, error) { return h.expandAlias(u) } -func (h *Headscale) generateACLPolicyDestPorts(d string) (*[]tailcfg.NetPortRange, error) { +func (h *Headscale) generateACLPolicyDestPorts(d string) ([]tailcfg.NetPortRange, error) { tokens := strings.Split(d, ":") if len(tokens) < 2 || len(tokens) > 3 { return nil, errorInvalidPortFormat @@ -128,7 +130,7 @@ func (h *Headscale) generateACLPolicyDestPorts(d string) (*[]tailcfg.NetPortRang } dests := []tailcfg.NetPortRange{} - for _, d := range *expanded { + for _, d := range expanded { for _, p := range *ports { pr := tailcfg.NetPortRange{ IP: d, @@ -137,12 +139,12 @@ func (h *Headscale) generateACLPolicyDestPorts(d string) (*[]tailcfg.NetPortRang dests = append(dests, pr) } } - return &dests, nil + return dests, nil } -func (h *Headscale) expandAlias(s string) (*[]string, error) { +func (h *Headscale) expandAlias(s string) ([]string, error) { if s == "*" { - return &[]string{"*"}, nil + return []string{"*"}, nil } if strings.HasPrefix(s, "group:") { @@ -155,11 +157,11 @@ func (h *Headscale) expandAlias(s string) (*[]string, error) { if err != nil { return nil, errorInvalidNamespace } - for _, node := range *nodes { + for _, node := range nodes { ips = append(ips, node.IPAddress) } } - return &ips, nil + return ips, nil } if strings.HasPrefix(s, "tag:") { @@ -195,7 +197,7 @@ func (h *Headscale) expandAlias(s string) (*[]string, error) { } } } - return &ips, nil + return ips, nil } n, err := h.GetNamespace(s) @@ -205,24 +207,24 @@ func (h *Headscale) expandAlias(s string) (*[]string, error) { return nil, err } ips := []string{} - for _, n := range *nodes { + for _, n := range nodes { ips = append(ips, n.IPAddress) } - return &ips, nil + return ips, nil } if h, ok := h.aclPolicy.Hosts[s]; ok { - return &[]string{h.String()}, nil + return []string{h.String()}, nil } ip, err := netaddr.ParseIP(s) if err == nil { - return &[]string{ip.String()}, nil + return []string{ip.String()}, nil } cidr, err := netaddr.ParseIPPrefix(s) if err == nil { - return &[]string{cidr.String()}, nil + return []string{cidr.String()}, nil } return nil, errorInvalidUserSection diff --git a/acls_test.go b/acls_test.go index dc5b4b31..da7f3ecc 100644 --- a/acls_test.go +++ b/acls_test.go @@ -12,7 +12,6 @@ func (s *Suite) TestWrongPath(c *check.C) { func (s *Suite) TestBrokenHuJson(c *check.C) { err := h.LoadACLPolicy("./tests/acls/broken.hujson") c.Assert(err, check.NotNil) - } func (s *Suite) TestInvalidPolicyHuson(c *check.C) { @@ -57,10 +56,10 @@ func (s *Suite) TestPortRange(c *check.C) { c.Assert(err, check.IsNil) c.Assert(rules, check.NotNil) - c.Assert(*rules, check.HasLen, 1) - c.Assert((*rules)[0].DstPorts, check.HasLen, 1) - c.Assert((*rules)[0].DstPorts[0].Ports.First, check.Equals, uint16(5400)) - c.Assert((*rules)[0].DstPorts[0].Ports.Last, check.Equals, uint16(5500)) + c.Assert(rules, check.HasLen, 1) + c.Assert((rules)[0].DstPorts, check.HasLen, 1) + c.Assert((rules)[0].DstPorts[0].Ports.First, check.Equals, uint16(5400)) + c.Assert((rules)[0].DstPorts[0].Ports.Last, check.Equals, uint16(5500)) } func (s *Suite) TestPortWildcard(c *check.C) { @@ -71,12 +70,12 @@ func (s *Suite) TestPortWildcard(c *check.C) { c.Assert(err, check.IsNil) c.Assert(rules, check.NotNil) - c.Assert(*rules, check.HasLen, 1) - c.Assert((*rules)[0].DstPorts, check.HasLen, 1) - c.Assert((*rules)[0].DstPorts[0].Ports.First, check.Equals, uint16(0)) - c.Assert((*rules)[0].DstPorts[0].Ports.Last, check.Equals, uint16(65535)) - c.Assert((*rules)[0].SrcIPs, check.HasLen, 1) - c.Assert((*rules)[0].SrcIPs[0], check.Equals, "*") + c.Assert(rules, check.HasLen, 1) + c.Assert((rules)[0].DstPorts, check.HasLen, 1) + c.Assert((rules)[0].DstPorts[0].Ports.First, check.Equals, uint16(0)) + c.Assert((rules)[0].DstPorts[0].Ports.Last, check.Equals, uint16(65535)) + c.Assert((rules)[0].SrcIPs, check.HasLen, 1) + c.Assert((rules)[0].SrcIPs[0], check.Equals, "*") } func (s *Suite) TestPortNamespace(c *check.C) { @@ -110,13 +109,13 @@ func (s *Suite) TestPortNamespace(c *check.C) { c.Assert(err, check.IsNil) c.Assert(rules, check.NotNil) - c.Assert(*rules, check.HasLen, 1) - c.Assert((*rules)[0].DstPorts, check.HasLen, 1) - c.Assert((*rules)[0].DstPorts[0].Ports.First, check.Equals, uint16(0)) - c.Assert((*rules)[0].DstPorts[0].Ports.Last, check.Equals, uint16(65535)) - c.Assert((*rules)[0].SrcIPs, check.HasLen, 1) - c.Assert((*rules)[0].SrcIPs[0], check.Not(check.Equals), "not an ip") - c.Assert((*rules)[0].SrcIPs[0], check.Equals, ip.String()) + c.Assert(rules, check.HasLen, 1) + c.Assert((rules)[0].DstPorts, check.HasLen, 1) + c.Assert((rules)[0].DstPorts[0].Ports.First, check.Equals, uint16(0)) + c.Assert((rules)[0].DstPorts[0].Ports.Last, check.Equals, uint16(65535)) + c.Assert((rules)[0].SrcIPs, check.HasLen, 1) + c.Assert((rules)[0].SrcIPs[0], check.Not(check.Equals), "not an ip") + c.Assert((rules)[0].SrcIPs[0], check.Equals, ip.String()) } func (s *Suite) TestPortGroup(c *check.C) { @@ -150,11 +149,11 @@ func (s *Suite) TestPortGroup(c *check.C) { c.Assert(err, check.IsNil) c.Assert(rules, check.NotNil) - c.Assert(*rules, check.HasLen, 1) - c.Assert((*rules)[0].DstPorts, check.HasLen, 1) - c.Assert((*rules)[0].DstPorts[0].Ports.First, check.Equals, uint16(0)) - c.Assert((*rules)[0].DstPorts[0].Ports.Last, check.Equals, uint16(65535)) - c.Assert((*rules)[0].SrcIPs, check.HasLen, 1) - c.Assert((*rules)[0].SrcIPs[0], check.Not(check.Equals), "not an ip") - c.Assert((*rules)[0].SrcIPs[0], check.Equals, ip.String()) + c.Assert(rules, check.HasLen, 1) + c.Assert((rules)[0].DstPorts, check.HasLen, 1) + c.Assert((rules)[0].DstPorts[0].Ports.First, check.Equals, uint16(0)) + c.Assert((rules)[0].DstPorts[0].Ports.Last, check.Equals, uint16(65535)) + c.Assert((rules)[0].SrcIPs, check.HasLen, 1) + c.Assert((rules)[0].SrcIPs[0], check.Not(check.Equals), "not an ip") + c.Assert((rules)[0].SrcIPs[0], check.Equals, ip.String()) } diff --git a/oidc_test.go b/oidc_test.go index b501ff14..c7a29ce9 100644 --- a/oidc_test.go +++ b/oidc_test.go @@ -22,7 +22,7 @@ func TestHeadscale_getNamespaceFromEmail(t *testing.T) { publicKey *wgkey.Key privateKey *wgkey.Private aclPolicy *ACLPolicy - aclRules *[]tailcfg.FilterRule + aclRules []tailcfg.FilterRule lastStateChange sync.Map oidcProvider *oidc.Provider oauth2Config *oauth2.Config diff --git a/sharing_test.go b/sharing_test.go index 1133fd92..4d9e4092 100644 --- a/sharing_test.go +++ b/sharing_test.go @@ -35,8 +35,20 @@ func CreateNodeNamespace(c *check.C, namespace, node, key, IP string) (*Namespac } func (s *Suite) TestBasicSharedNodesInNamespace(c *check.C) { - n1, m1 := CreateNodeNamespace(c, "shared1", "test_get_shared_nodes_1", "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", "100.64.0.1") - _, m2 := CreateNodeNamespace(c, "shared2", "test_get_shared_nodes_2", "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", "100.64.0.2") + n1, m1 := CreateNodeNamespace( + c, + "shared1", + "test_get_shared_nodes_1", + "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", + "100.64.0.1", + ) + _, m2 := CreateNodeNamespace( + c, + "shared2", + "test_get_shared_nodes_2", + "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", + "100.64.0.2", + ) p1s, err := h.getPeers(m1) c.Assert(err, check.IsNil) @@ -52,7 +64,13 @@ func (s *Suite) TestBasicSharedNodesInNamespace(c *check.C) { } func (s *Suite) TestSameNamespace(c *check.C) { - n1, m1 := CreateNodeNamespace(c, "shared1", "test_get_shared_nodes_1", "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", "100.64.0.1") + n1, m1 := CreateNodeNamespace( + c, + "shared1", + "test_get_shared_nodes_1", + "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", + "100.64.0.1", + ) p1s, err := h.getPeers(m1) c.Assert(err, check.IsNil) @@ -63,8 +81,20 @@ func (s *Suite) TestSameNamespace(c *check.C) { } func (s *Suite) TestUnshare(c *check.C) { - n1, m1 := CreateNodeNamespace(c, "shared1", "test_unshare_1", "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", "100.64.0.1") - _, m2 := CreateNodeNamespace(c, "shared2", "test_unshare_2", "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", "100.64.0.2") + n1, m1 := CreateNodeNamespace( + c, + "shared1", + "test_unshare_1", + "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", + "100.64.0.1", + ) + _, m2 := CreateNodeNamespace( + c, + "shared2", + "test_unshare_2", + "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", + "100.64.0.2", + ) p1s, err := h.getPeers(m1) c.Assert(err, check.IsNil) @@ -92,8 +122,20 @@ func (s *Suite) TestUnshare(c *check.C) { } func (s *Suite) TestAlreadyShared(c *check.C) { - n1, m1 := CreateNodeNamespace(c, "shared1", "test_get_shared_nodes_1", "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", "100.64.0.1") - _, m2 := CreateNodeNamespace(c, "shared2", "test_get_shared_nodes_2", "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", "100.64.0.2") + n1, m1 := CreateNodeNamespace( + c, + "shared1", + "test_get_shared_nodes_1", + "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", + "100.64.0.1", + ) + _, m2 := CreateNodeNamespace( + c, + "shared2", + "test_get_shared_nodes_2", + "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", + "100.64.0.2", + ) p1s, err := h.getPeers(m1) c.Assert(err, check.IsNil) @@ -106,8 +148,20 @@ func (s *Suite) TestAlreadyShared(c *check.C) { } func (s *Suite) TestDoNotIncludeRoutesOnShared(c *check.C) { - n1, m1 := CreateNodeNamespace(c, "shared1", "test_get_shared_nodes_1", "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", "100.64.0.1") - _, m2 := CreateNodeNamespace(c, "shared2", "test_get_shared_nodes_2", "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", "100.64.0.2") + n1, m1 := CreateNodeNamespace( + c, + "shared1", + "test_get_shared_nodes_1", + "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", + "100.64.0.1", + ) + _, m2 := CreateNodeNamespace( + c, + "shared2", + "test_get_shared_nodes_2", + "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", + "100.64.0.2", + ) p1s, err := h.getPeers(m1) c.Assert(err, check.IsNil) @@ -123,9 +177,27 @@ func (s *Suite) TestDoNotIncludeRoutesOnShared(c *check.C) { } func (s *Suite) TestComplexSharingAcrossNamespaces(c *check.C) { - n1, m1 := CreateNodeNamespace(c, "shared1", "test_get_shared_nodes_1", "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", "100.64.0.1") - _, m2 := CreateNodeNamespace(c, "shared2", "test_get_shared_nodes_2", "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", "100.64.0.2") - _, m3 := CreateNodeNamespace(c, "shared3", "test_get_shared_nodes_3", "6e704bee83eb93db6fc2c417d7882964cd3f8cc87082cbb645982e34020c76c8", "100.64.0.3") + n1, m1 := CreateNodeNamespace( + c, + "shared1", + "test_get_shared_nodes_1", + "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", + "100.64.0.1", + ) + _, m2 := CreateNodeNamespace( + c, + "shared2", + "test_get_shared_nodes_2", + "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", + "100.64.0.2", + ) + _, m3 := CreateNodeNamespace( + c, + "shared3", + "test_get_shared_nodes_3", + "6e704bee83eb93db6fc2c417d7882964cd3f8cc87082cbb645982e34020c76c8", + "100.64.0.3", + ) pak4, err := h.CreatePreAuthKey(n1.Name, false, false, nil) c.Assert(err, check.IsNil) @@ -172,15 +244,37 @@ func (s *Suite) TestComplexSharingAcrossNamespaces(c *check.C) { pSharedTo, err := h.getPeers(m2) c.Assert(err, check.IsNil) - c.Assert(len(pSharedTo), check.Equals, 2) // node2 should see node1 (sharedTo) and node4 (sharedTo), as is shared in namespace1 + c.Assert( + len(pSharedTo), + check.Equals, + 2, + ) // node2 should see node1 (sharedTo) and node4 (sharedTo), as is shared in namespace1 c.Assert(pSharedTo[0].Name, check.Equals, m1.Name) c.Assert(pSharedTo[1].Name, check.Equals, m4.Name) } func (s *Suite) TestDeleteSharedMachine(c *check.C) { - n1, m1 := CreateNodeNamespace(c, "shared1", "test_get_shared_nodes_1", "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", "100.64.0.1") - _, m2 := CreateNodeNamespace(c, "shared2", "test_get_shared_nodes_2", "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", "100.64.0.2") - _, m3 := CreateNodeNamespace(c, "shared3", "test_get_shared_nodes_3", "6e704bee83eb93db6fc2c417d7882964cd3f8cc87082cbb645982e34020c76c8", "100.64.0.3") + n1, m1 := CreateNodeNamespace( + c, + "shared1", + "test_get_shared_nodes_1", + "686824e749f3b7f2a5927ee6c1e422aee5292592d9179a271ed7b3e659b44a66", + "100.64.0.1", + ) + _, m2 := CreateNodeNamespace( + c, + "shared2", + "test_get_shared_nodes_2", + "dec46ef9dc45c7d2f03bfcd5a640d9e24e3cc68ce3d9da223867c9bc6d5e9863", + "100.64.0.2", + ) + _, m3 := CreateNodeNamespace( + c, + "shared3", + "test_get_shared_nodes_3", + "6e704bee83eb93db6fc2c417d7882964cd3f8cc87082cbb645982e34020c76c8", + "100.64.0.3", + ) pak4n1, err := h.CreatePreAuthKey(n1.Name, false, false, nil) c.Assert(err, check.IsNil) @@ -226,12 +320,12 @@ func (s *Suite) TestDeleteSharedMachine(c *check.C) { sharedMachines, err := h.ListSharedMachinesInNamespace(n1.Name) c.Assert(err, check.IsNil) - c.Assert(len(*sharedMachines), check.Equals, 1) + c.Assert(len(sharedMachines), check.Equals, 1) err = h.DeleteMachine(m2) c.Assert(err, check.IsNil) sharedMachines, err = h.ListSharedMachinesInNamespace(n1.Name) c.Assert(err, check.IsNil) - c.Assert(len(*sharedMachines), check.Equals, 0) + c.Assert(len(sharedMachines), check.Equals, 0) }