Skip to content

Commit

Permalink
Merge pull request #143 from rammanoj/firewall_support_v2
Browse files Browse the repository at this point in the history 
Upgrade Linodego dependency to the latest available version v1.25.0 and fix the existing unit-test failures due to the changes.
Add support to attach firewall to node balancer when creating a node balancer.
Add corresponding unit-tests:
1. Create NodeBalancer without a firewall
2. Create NodeBalancer with an invalid firewall ID
3. Create NodeBalaner with a valid firewall ID
  • Loading branch information
@tchinmai7
tchinmai7 authored Dec 4, 2023
2 parents 4084b5e + 15e5f7a commit 8148850
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 68 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ Annotation (Suffix) | Values | Default | Description
`nodebalancer-id` | string | | The ID of the NodeBalancer to front the service. When not specified, a new NodeBalancer will be created. This can be configured on service creation or patching
`hostname-only-ingress` | [bool](#annotation-bool-values) | `false` | When `true`, the LoadBalancerStatus for the service will only contain the Hostname. This is useful for bypassing kube-proxy's rerouting of in-cluster requests originally intended for the external LoadBalancer to the service's constituent pod IPs.
`tags` | string | | A comma seperated list of tags to be applied to the createad NodeBalancer instance
`firewall-id` | string | | The Firewall ID that's applied to the NodeBalancer instance.

#### Deprecated Annotations

Expand Down
44 changes: 26 additions & 18 deletions cloud/linode/fake_linode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
"github.com/linode/linodego"
)

const apiVersion = "v4"

type fakeAPI struct {
t *testing.T
nb map[string]*linodego.NodeBalancer
Expand Down Expand Up @@ -46,12 +48,12 @@ func (f *fakeAPI) ResetRequests() {
f.requests = make(map[fakeRequest]struct{})
}

func (f *fakeAPI) recordRequest(r *http.Request) {
func (f *fakeAPI) recordRequest(r *http.Request, urlPath string) {
bodyBytes, _ := ioutil.ReadAll(r.Body)
r.Body.Close()
r.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes))
f.requests[fakeRequest{
Path: r.URL.Path,
Path: urlPath,
Method: r.Method,
Body: string(bodyBytes),
}] = struct{}{}
Expand All @@ -67,10 +69,16 @@ func (f *fakeAPI) didRequestOccur(method, path, body string) bool {
}

func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
f.recordRequest(r)

w.Header().Set("Content-Type", "application/json")
urlPath := r.URL.Path

if !strings.HasPrefix(urlPath, "/"+apiVersion) {
http.Error(w, "not found", http.StatusNotFound)
return
}
urlPath = strings.TrimPrefix(urlPath, "/"+apiVersion)
f.recordRequest(r, urlPath)

switch r.Method {
case "GET":
whichAPI := strings.Split(urlPath[1:], "/")
Expand Down Expand Up @@ -99,7 +107,7 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
rx, _ = regexp.Compile("/nodebalancers/[0-9]+/configs/[0-9]+/nodes")
if rx.MatchString(urlPath) {
res := 0
parts := strings.Split(r.URL.Path[1:], "/")
parts := strings.Split(urlPath[1:], "/")
nbcID, err := strconv.Atoi(parts[3])
if err != nil {
f.t.Fatal(err)
Expand Down Expand Up @@ -236,7 +244,7 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}

case "POST":
tp := filepath.Base(r.URL.Path)
tp := filepath.Base(urlPath)
if tp == "nodebalancers" {
nbco := linodego.NodeBalancerCreateOptions{}
if err := json.NewDecoder(r.Body).Decode(&nbco); err != nil {
Expand Down Expand Up @@ -313,7 +321,7 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return

} else if tp == "rebuild" {
parts := strings.Split(r.URL.Path[1:], "/")
parts := strings.Split(urlPath[1:], "/")
nbcco := new(linodego.NodeBalancerConfigRebuildOptions)
if err := json.NewDecoder(r.Body).Decode(nbcco); err != nil {
f.t.Fatal(err)
Expand Down Expand Up @@ -382,7 +390,7 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write(resp)
return
} else if tp == "configs" {
parts := strings.Split(r.URL.Path[1:], "/")
parts := strings.Split(urlPath[1:], "/")
nbcco := new(linodego.NodeBalancerConfigCreateOptions)
if err := json.NewDecoder(r.Body).Decode(nbcco); err != nil {
f.t.Fatal(err)
Expand Down Expand Up @@ -422,7 +430,7 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
_, _ = w.Write(resp)
return
} else if tp == "nodes" {
parts := strings.Split(r.URL.Path[1:], "/")
parts := strings.Split(urlPath[1:], "/")
nbnco := new(linodego.NodeBalancerNodeCreateOptions)
if err := json.NewDecoder(r.Body).Decode(nbnco); err != nil {
f.t.Fatal(err)
Expand Down Expand Up @@ -454,22 +462,22 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}
case "DELETE":
idRaw := filepath.Base(r.URL.Path)
idRaw := filepath.Base(urlPath)
id, err := strconv.Atoi(idRaw)
if err != nil {
f.t.Fatal(err)
}
if strings.Contains(r.URL.Path, "nodes") {
if strings.Contains(urlPath, "nodes") {
delete(f.nbn, idRaw)
} else if strings.Contains(r.URL.Path, "configs") {
} else if strings.Contains(urlPath, "configs") {
delete(f.nbc, idRaw)

for k, n := range f.nbn {
if n.ConfigID == id {
delete(f.nbn, k)
}
}
} else if strings.Contains(r.URL.Path, "nodebalancers") {
} else if strings.Contains(urlPath, "nodebalancers") {
delete(f.nb, idRaw)

for k, c := range f.nbc {
Expand All @@ -485,10 +493,10 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
}
case "PUT":
if strings.Contains(r.URL.Path, "nodes") {
if strings.Contains(urlPath, "nodes") {
f.t.Fatal("PUT ...nodes is not supported by the mock API")
} else if strings.Contains(r.URL.Path, "configs") {
parts := strings.Split(r.URL.Path[1:], "/")
} else if strings.Contains(urlPath, "configs") {
parts := strings.Split(urlPath[1:], "/")
nbcco := new(linodego.NodeBalancerConfigUpdateOptions)
if err := json.NewDecoder(r.Body).Decode(nbcco); err != nil {
f.t.Fatal(err)
Expand Down Expand Up @@ -545,8 +553,8 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
_, _ = w.Write(resp)
return
} else if strings.Contains(r.URL.Path, "nodebalancer") {
parts := strings.Split(r.URL.Path[1:], "/")
} else if strings.Contains(urlPath, "nodebalancer") {
parts := strings.Split(urlPath[1:], "/")
nbuo := new(linodego.NodeBalancerUpdateOptions)
if err := json.NewDecoder(r.Body).Decode(nbuo); err != nil {
f.t.Fatal(err)
Expand Down
10 changes: 10 additions & 0 deletions cloud/linode/loadbalancers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ const (

annLinodeHostnameOnlyIngress = "service.beta.kubernetes.io/linode-loadbalancer-hostname-only-ingress"
annLinodeLoadBalancerTags = "service.beta.kubernetes.io/linode-loadbalancer-tags"
annLinodeCloudFirewallID = "service.beta.kubernetes.io/linode-loadbalancer-firewall-id"

annLinodeNodePrivateIP = "node.k8s.linode.com/private-ip"
)
Expand Down Expand Up @@ -524,6 +525,15 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri
Configs: configs,
Tags: tags,
}

fwid, ok := getServiceAnnotation(service, annLinodeCloudFirewallID)
if ok {
firewallID, err := strconv.Atoi(fwid)
if err != nil {
return nil, err
}
createOpts.FirewallID = firewallID
}
return l.client.CreateNodeBalancer(ctx, createOpts)
}

Expand Down
60 changes: 45 additions & 15 deletions cloud/linode/loadbalancers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,16 @@ func TestCCMLoadBalancers(t *testing.T) {
f: testGetLoadBalancer,
},
{
name: "Create Load Balancer",
f: testCreateNodeBalancer,
name: "Create Load Balancer Without Firewall",
f: testCreateNodeBalancerWithOutFirewall,
},
{
name: "Create Load Balancer With Valid Firewall ID",
f: testCreateNodeBalancerWithFirewall,
},
{
name: "Create Load Balancer With Invalid Firewall ID",
f: testCreateNodeBalancerWithInvalidFirewall,
},
{
name: "Update Load Balancer - Add Annotation",
Expand Down Expand Up @@ -205,7 +213,7 @@ func stubService(fake *fake.Clientset, service *v1.Service) {
fake.CoreV1().Services("").Create(context.TODO(), service, metav1.CreateOptions{})
}

func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) {
func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI, firewallID *string) error {
svc := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: randString(10),
Expand Down Expand Up @@ -233,14 +241,19 @@ func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) {
},
}

if firewallID != nil {
svc.Annotations[annLinodeCloudFirewallID] = *firewallID
}

lb := &loadbalancers{client, "us-west", nil}
nodes := []*v1.Node{
{ObjectMeta: metav1.ObjectMeta{Name: "node-1"}},
}
nb, err := lb.buildLoadBalancerRequest(context.TODO(), "linodelb", svc, nodes)
if err != nil {
t.Fatal(err)
return err
}

if nb.Region != lb.zone {
t.Error("unexpected nodebalancer region")
t.Logf("expected: %s", lb.zone)
Expand All @@ -249,7 +262,7 @@ func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) {

configs, err := client.ListNodeBalancerConfigs(context.TODO(), nb.ID, nil)
if err != nil {
t.Fatal(err)
return err
}

if len(configs) != len(svc.Spec.Ports) {
Expand All @@ -258,17 +271,9 @@ func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) {
t.Logf("actual: %v", len(configs))
}

if !reflect.DeepEqual(err, nil) {
t.Error("unexpected error")
t.Logf("expected: %v", nil)
t.Logf("actual: %v", err)
}

nb, err = client.GetNodeBalancer(context.TODO(), nb.ID)
if !reflect.DeepEqual(err, nil) {
t.Error("unexpected error")
t.Logf("expected: %v", nil)
t.Logf("actual: %v", err)
if err != nil {
return err
}

if nb.ClientConnThrottle != 15 {
Expand All @@ -285,6 +290,31 @@ func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) {
}

defer func() { _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) }()
return nil
}

func testCreateNodeBalancerWithOutFirewall(t *testing.T, client *linodego.Client, f *fakeAPI) {
err := testCreateNodeBalancer(t, client, f, nil)
if err != nil {
t.Fatalf("expected a nil error, got %v", err)
}
}

func testCreateNodeBalancerWithFirewall(t *testing.T, client *linodego.Client, f *fakeAPI) {
firewallID := "123"
err := testCreateNodeBalancer(t, client, f, &firewallID)
if err != nil {
t.Fatalf("expected a nil error, got %v", err)
}
}

func testCreateNodeBalancerWithInvalidFirewall(t *testing.T, client *linodego.Client, f *fakeAPI) {
firewallID := "qwerty"
expectedError := "strconv.Atoi: parsing \"qwerty\": invalid syntax"
err := testCreateNodeBalancer(t, client, f, &firewallID)
if err.Error() != expectedError {
t.Fatalf("expected a %s error, got %v", expectedError, err)
}
}

func testUpdateLoadBalancerAddAnnotation(t *testing.T, client *linodego.Client, _ *fakeAPI) {
Expand Down
32 changes: 16 additions & 16 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@ require (
github.com/appscode/go v0.0.0-20200323182826-54e98e09185a
github.com/getsentry/sentry-go v0.4.0
github.com/golang/mock v1.6.0
github.com/linode/linodego v0.32.2
github.com/pkg/errors v0.9.1
github.com/linode/linodego v1.25.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.7.0
github.com/stretchr/testify v1.8.4
k8s.io/api v0.21.0
k8s.io/apimachinery v0.21.0
k8s.io/client-go v0.21.0
Expand Down Expand Up @@ -39,11 +38,11 @@ require (
github.com/go-openapi/jsonreference v0.19.3 // indirect
github.com/go-openapi/spec v0.19.5 // indirect
github.com/go-openapi/swag v0.19.5 // indirect
github.com/go-resty/resty/v2 v2.1.1-0.20191201195748-d7b97669fe48 // indirect
github.com/go-resty/resty/v2 v2.9.1 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect
github.com/golang/protobuf v1.4.3 // indirect
github.com/google/go-cmp v0.5.2 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/gofuzz v1.1.0 // indirect
github.com/google/uuid v1.1.2 // indirect
github.com/googleapis/gnostic v0.4.1 // indirect
Expand All @@ -59,6 +58,7 @@ require (
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.1 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_golang v1.7.1 // indirect
github.com/prometheus/client_model v0.2.0 // indirect
Expand All @@ -70,26 +70,26 @@ require (
go.uber.org/multierr v1.3.0 // indirect
go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee // indirect
go.uber.org/zap v1.13.0 // indirect
golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83 // indirect
golang.org/x/crypto v0.14.0 // indirect
golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect
golang.org/x/mod v0.4.2 // indirect
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4 // indirect
golang.org/x/mod v0.8.0 // indirect
golang.org/x/net v0.17.0 // indirect
golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d // indirect
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect
golang.org/x/sys v0.0.0-20210510120138-977fb7262007 // indirect
golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d // indirect
golang.org/x/text v0.3.4 // indirect
golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba // indirect
golang.org/x/tools v0.1.1 // indirect
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect
golang.org/x/sync v0.1.0 // indirect
golang.org/x/sys v0.13.0 // indirect
golang.org/x/term v0.13.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/time v0.0.0-20211116232009-f0f3c7e86c11 // indirect
golang.org/x/tools v0.6.0 // indirect
google.golang.org/appengine v1.6.5 // indirect
google.golang.org/genproto v0.0.0-20201110150050-8816d57aaa9a // indirect
google.golang.org/grpc v1.27.1 // indirect
google.golang.org/protobuf v1.25.0 // indirect
gopkg.in/inf.v0 v0.9.1 // indirect
gopkg.in/ini.v1 v1.66.6 // indirect
gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
honnef.co/go/tools v0.0.1-2020.1.3 // indirect
k8s.io/apiserver v0.21.0 // indirect
k8s.io/controller-manager v0.21.0 // indirect
Expand Down
Loading

0 comments on commit 8148850

Please sign in to comment.