From 90267506046d75123e268006af61d9bdc4e4654f Mon Sep 17 00:00:00 2001 From: Pedro Coutinho Date: Wed, 12 Jul 2023 13:47:28 -0700 Subject: [PATCH] Fix panic in 'calicoctl get nodes' 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', https://github.com/projectcalico/calico/issues/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. --- calicoctl/Makefile | 2 +- .../commands/common/common_suite_test.go | 13 +++ .../calicoctl/commands/common/printer.go | 8 +- .../calicoctl/commands/common/printer_test.go | 107 ++++++++++++++++++ 4 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 calicoctl/calicoctl/commands/common/common_suite_test.go diff --git a/calicoctl/Makefile b/calicoctl/Makefile index b3979c3d88c..b96677410e4 100644 --- a/calicoctl/Makefile +++ b/calicoctl/Makefile @@ -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 diff --git a/calicoctl/calicoctl/commands/common/common_suite_test.go b/calicoctl/calicoctl/commands/common/common_suite_test.go new file mode 100644 index 00000000000..690b5549391 --- /dev/null +++ b/calicoctl/calicoctl/commands/common/common_suite_test.go @@ -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") +} diff --git a/calicoctl/calicoctl/commands/common/printer.go b/calicoctl/calicoctl/commands/common/printer.go index 75fc5a89235..6c4fbf3f196 100644 --- a/calicoctl/calicoctl/commands/common/printer.go +++ b/calicoctl/calicoctl/commands/common/printer.go @@ -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 diff --git a/calicoctl/calicoctl/commands/common/printer_test.go b/calicoctl/calicoctl/commands/common/printer_test.go index 8eb384267b9..5a73c81281d 100644 --- a/calicoctl/calicoctl/commands/common/printer_test.go +++ b/calicoctl/calicoctl/commands/common/printer_test.go @@ -1,8 +1,35 @@ +// Copyright (c) 2019-2023 Tigera, Inc. All rights reserved. + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + 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 @@ -37,3 +64,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 +}