Skip to content

Commit

Permalink
Fix panic in 'calicoctl get nodes'
Browse files Browse the repository at this point in the history
Fix panic when bgpConfig.Spec.ASNumber was nil in calicoctl/calicoctl/commands/common/printer.go (used by calicoctl/calicoctl/resourcemgr/node.go when running 'calicoctl get nodes', #7755 for more info).

Add printer config() test to calicoctl/calicoctl/commands/common/printer_test.go.

Run ginkgo bootstrap in calicoctl/calicoctl/commands/common/ (tests were not being run previously).

Consider $(GINKGO_ARGS) when running the 'ut' target in calicoctl/Makefile.
  • Loading branch information
coutinhop committed Jul 12, 2023
1 parent 23440dc commit 9866a54
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 2 deletions.
2 changes: 1 addition & 1 deletion calicoctl/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ $(CALICO_VERSION_HELPER_BIN): $(CALICO_VERSION_HELPER_SRC)
.PHONY: ut
## Run the tests in a container. Useful for CI, Mac dev.
ut: bin/calicoctl-linux-amd64
$(DOCKER_RUN) $(CALICO_BUILD) sh -c 'cd /go/src/$(PACKAGE_NAME) && ginkgo -cover -r calicoctl/*'
$(DOCKER_RUN) $(CALICO_BUILD) sh -c 'cd /go/src/$(PACKAGE_NAME) && ginkgo -cover -r $(GINKGO_ARGS) calicoctl/*'

###############################################################################
# FVs
Expand Down
13 changes: 13 additions & 0 deletions calicoctl/calicoctl/commands/common/common_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package common_test

import (
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

func TestCommon(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Common Suite")
}
8 changes: 7 additions & 1 deletion calicoctl/calicoctl/commands/common/printer.go
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,13 @@ func config(client client.Interface) func(string) string {
asValue = "64512"
}
} else {
asValue = bgpConfig.Spec.ASNumber.String()
if bgpConfig.Spec.ASNumber != nil {
asValue = bgpConfig.Spec.ASNumber.String()
} else {
// Use the default ASNumber of 64512 when there is none configured (first ASN reserved for private use).
// https://en.m.wikipedia.org/wiki/Autonomous_system_(Internet)#ASN_Table
asValue = "64512"
}
}
}
return asValue
Expand Down
92 changes: 92 additions & 0 deletions calicoctl/calicoctl/commands/common/printer_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,20 @@
package common

import (
"context"
"errors"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

apiv3 "github.com/projectcalico/api/pkg/apis/projectcalico/v3"
"github.com/projectcalico/api/pkg/lib/numorstring"
"github.com/projectcalico/calico/libcalico-go/lib/clientv3"
cerrors "github.com/projectcalico/calico/libcalico-go/lib/errors"
"github.com/projectcalico/calico/libcalico-go/lib/options"
)

var nilSlice []int
Expand Down Expand Up @@ -37,3 +49,83 @@ var _ = DescribeTable("Testing joinAndTruncate",
Entry("slice no truncate", []int{123456}, ",", 6, "123456"),
Entry("string", "HelloWorld", ",", 0, "HelloWorld"),
)

var _ = Describe("Testing printer config()", func() {
var client *mockClient
BeforeEach(func() {
client = newMockClient()
})

It("prints the default asnumber 64512 when BGPConfig.Spec.ASNumber is nil", func() {
Expect(config(client)("asnumber")).To(Equal("64512"))
})

It("prints the default asnumber 64512 when the default BGPConfig cannot be found", func() {
client.bgpConfig = nil
Expect(config(client)("asnumber")).To(Equal("64512"))
})

It("prints the asnumber from BGPConfig.Spec.ASNumber when it is present", func() {
asNumber := numorstring.ASNumber(12345)
bgpConfig := apiv3.BGPConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: "default", ResourceVersion: "1234",
CreationTimestamp: metav1.Now(),
UID: "test-printer-bgpconfig",
},
Spec: apiv3.BGPConfigurationSpec{
ASNumber: &asNumber,
},
}
client.BGPConfigurations().Update(context.Background(), &bgpConfig, options.SetOptions{})
Expect(config(client)("asnumber")).To(Equal(asNumber.String()))
})

It("prints 'unknown' when there is an error getting the default BGPConfig", func() {
client.throwError = true
Expect(config(client)("asnumber")).To(Equal("unknown"))
})
})

type mockClient struct {
clientv3.Interface
clientv3.BGPConfigurationInterface
bgpConfig *apiv3.BGPConfiguration
throwError bool
}

func newMockClient() *mockClient {
return &mockClient{
bgpConfig: &apiv3.BGPConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: "default", ResourceVersion: "1234",
CreationTimestamp: metav1.Now(),
UID: "test-printer-bgpconfig",
},
Spec: apiv3.BGPConfigurationSpec{},
},
throwError: false,
}
}

func (c *mockClient) BGPConfigurations() clientv3.BGPConfigurationInterface {
return c
}

func (c *mockClient) Get(_ context.Context, name string, _ options.GetOptions) (*apiv3.BGPConfiguration, error) {
if c.throwError {
return nil, errors.New("mock error for testing")
}

if c.bgpConfig == nil {
return nil, cerrors.ErrorResourceDoesNotExist{}

}

return c.bgpConfig, nil
}

func (c *mockClient) Update(_ context.Context, res *apiv3.BGPConfiguration, _ options.SetOptions) (*apiv3.BGPConfiguration, error) {
c.bgpConfig = res
return c.bgpConfig, nil
}

0 comments on commit 9866a54

Please sign in to comment.