diff --git a/pkg/log/log_suite_test.go b/pkg/log/log_suite_test.go new file mode 100644 index 00000000..bd3133e6 --- /dev/null +++ b/pkg/log/log_suite_test.go @@ -0,0 +1,79 @@ +/* +SPDX-License-Identifier: Apache-2.0 + +Copyright Contributors to the Submariner project. + +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 log_test + +import ( + "testing" + + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/submariner-io/admiral/pkg/log" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +func TestLog(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Log Suite") +} + +var ( + rootLogSink = newLogSinkImpl() + exited bool +) + +var _ = BeforeSuite(func() { + logf.SetLogger(logr.New(rootLogSink)) + + log.Exit = func(_ int) { + exited = true + } +}) + +type logSinkImpl struct { + infoCh chan []any + errorCh chan []any +} + +func newLogSinkImpl() *logSinkImpl { + return &logSinkImpl{infoCh: make(chan []any, 10), errorCh: make(chan []any, 10)} +} + +func (l *logSinkImpl) Init(_ logr.RuntimeInfo) { +} + +func (l *logSinkImpl) Enabled(_ int) bool { + return true +} + +func (l *logSinkImpl) Info(level int, msg string, keysAndValues ...any) { + l.infoCh <- append([]any{level, msg}, keysAndValues...) +} + +func (l *logSinkImpl) Error(err error, msg string, keysAndValues ...any) { + l.errorCh <- append([]any{err, msg}, keysAndValues...) +} + +func (l *logSinkImpl) WithValues(_ ...any) logr.LogSink { + return newLogSinkImpl() +} + +func (l *logSinkImpl) WithName(_ string) logr.LogSink { + return newLogSinkImpl() +} diff --git a/pkg/log/logger.go b/pkg/log/logger.go index ffe2a8c8..cc2dabe7 100644 --- a/pkg/log/logger.go +++ b/pkg/log/logger.go @@ -30,6 +30,9 @@ const ( FatalKey = "FATAL" ) +// Exit hook for unit tests. +var Exit = os.Exit + type Logger struct { logr.Logger } @@ -60,7 +63,7 @@ func (l Logger) Warningf(format string, args ...interface{}) { func (l Logger) Fatal(msg string, keysAndValues ...interface{}) { l.Logger.Error(nil, msg, append(keysAndValues, FatalKey, "true")...) - os.Exit(255) + Exit(255) } func (l Logger) Fatalf(format string, args ...interface{}) { @@ -73,7 +76,7 @@ func (l Logger) FatalOnError(err error, msg string, keysAndValues ...interface{} } l.Logger.Error(err, msg, append(keysAndValues, FatalKey, "true")...) - os.Exit(255) + Exit(255) } func (l Logger) FatalfOnError(err error, format string, args ...interface{}) { diff --git a/pkg/log/logger_test.go b/pkg/log/logger_test.go new file mode 100644 index 00000000..a2438635 --- /dev/null +++ b/pkg/log/logger_test.go @@ -0,0 +1,108 @@ +/* +SPDX-License-Identifier: Apache-2.0 + +Copyright Contributors to the Submariner project. + +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 log_test + +import ( + "errors" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/submariner-io/admiral/pkg/log" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +var _ = Describe("Logger", func() { + var logger log.Logger + + BeforeEach(func() { + exited = false + logger = log.Logger{Logger: logf.Log} + }) + + Specify("Infof should log a formatted info message", func() { + logger.Infof("value is %d", 3) + Expect(rootLogSink.infoCh).To(Receive(HaveExactElements(0, "value is 3"))) + }) + + Specify("Info should log an info message with keys and values", func() { + logger.Info("hello", "key", "value") + Expect(rootLogSink.infoCh).To(Receive(HaveExactElements(0, "hello", "key", "value"))) + }) + + Specify("Errorf should log the error with a formatted message", func() { + err := errors.New("some error") + logger.Errorf(err, "value is %d", 3) + Expect(rootLogSink.errorCh).To(Receive(HaveExactElements(err, "value is 3"))) + }) + + Specify("Error should log the error with a message and keys and values", func() { + err := errors.New("some error") + logger.Error(err, "failed", "key", "value") + Expect(rootLogSink.errorCh).To(Receive(HaveExactElements(err, "failed", "key", "value"))) + }) + + Specify("Warningf should log a formatted info message", func() { + logger.Warningf("value is %d", 3) + Expect(rootLogSink.infoCh).To(Receive(HaveExactElements(0, "value is 3", log.WarningKey, "true"))) + }) + + Specify("Warning should log an info message with keys and values", func() { + logger.Warning("hello", "key", "value") + Expect(rootLogSink.infoCh).To(Receive(HaveExactElements(0, "hello", "key", "value", log.WarningKey, "true"))) + }) + + Specify("Fatalf should log a formatted error message and then exit", func() { + logger.Fatalf("value is %d", 3) + Expect(rootLogSink.errorCh).To(Receive(HaveExactElements(nil, "value is 3", log.FatalKey, "true"))) + Expect(exited).To(BeTrue()) + }) + + Specify("Fatal should log an error message with keys and values and then exit", func() { + logger.Fatal("failed", "key", "value") + Expect(rootLogSink.errorCh).To(Receive(HaveExactElements(nil, "failed", "key", "value", log.FatalKey, "true"))) + Expect(exited).To(BeTrue()) + }) + + Context("FatalfOnError", func() { + Specify("with an error specified should log the error and then exit", func() { + err := errors.New("some error") + logger.FatalfOnError(err, "value is %d", 3) + Expect(rootLogSink.errorCh).To(Receive(HaveExactElements(err, "value is 3", log.FatalKey, "true"))) + Expect(exited).To(BeTrue()) + }) + + Specify("with no error specified should not exit", func() { + logger.FatalfOnError(nil, "value is %d", 3) + Expect(rootLogSink.errorCh).NotTo(Receive()) + Expect(exited).To(BeFalse()) + }) + }) + + Specify("FatalOnError should log the error and then exit", func() { + err := errors.New("some error") + logger.FatalOnError(err, "failed", "key", "value") + Expect(rootLogSink.errorCh).To(Receive(HaveExactElements(err, "failed", "key", "value", log.FatalKey, "true"))) + Expect(exited).To(BeTrue()) + }) + + Specify("V should set the log level", func() { + logger.V(2).Info("message") + Expect(rootLogSink.infoCh).To(Receive(HaveExactElements(2, "message"))) + }) +}) diff --git a/pkg/reporter/adapter_test.go b/pkg/reporter/adapter_test.go new file mode 100644 index 00000000..a4a6b873 --- /dev/null +++ b/pkg/reporter/adapter_test.go @@ -0,0 +1,76 @@ +/* +SPDX-License-Identifier: Apache-2.0 + +Copyright Contributors to the Submariner project. + +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 reporter_test + +import ( + "errors" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/submariner-io/admiral/pkg/reporter" +) + +type basicImpl struct { + reporter.Basic + endCalled chan struct{} +} + +func (b *basicImpl) End() { + close(b.endCalled) +} + +var _ = Describe("Adapter", func() { + stdoutCapture := newStdoutCapture() + + var adapter reporter.Adapter + + BeforeEach(func() { + adapter = reporter.Adapter{Basic: &basicImpl{Basic: reporter.Stdout(), endCalled: make(chan struct{})}} + stdoutCapture.start() + }) + + Context("Error", func() { + It("should log the error", func() { + err := errors.New("some error") + Expect(adapter.Error(err, "")).To(Equal(err)) + Expect(stdoutCapture.read()).To(ContainSubstring("Some error")) + Expect(adapter.Basic.(*basicImpl).endCalled).To(BeClosed()) + }) + + When("the error is nil", func() { + It("should not log anything", func() { + Expect(adapter.Error(nil, "failed")).To(Succeed()) + Expect(stdoutCapture.read()).To(BeEmpty()) + }) + }) + + When("a message is specified", func() { + It("should wrap the error with the message", func() { + err := errors.New("some error") + actual := adapter.Error(err, "failed") + + Expect(actual.Error()).To(HavePrefix("failed")) + + out := stdoutCapture.read() + Expect(out).To(ContainSubstring("Failed")) + Expect(out).To(ContainSubstring(err.Error())) + }) + }) + }) +}) diff --git a/pkg/reporter/reporter_suite_test.go b/pkg/reporter/reporter_suite_test.go new file mode 100644 index 00000000..d5cf5cb8 --- /dev/null +++ b/pkg/reporter/reporter_suite_test.go @@ -0,0 +1,74 @@ +/* +SPDX-License-Identifier: Apache-2.0 + +Copyright Contributors to the Submariner project. + +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 reporter_test + +import ( + "bytes" + "io" + "os" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestReporter(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Reporter Suite") +} + +type StdoutCapture struct { + oldStdout *os.File + readFile *os.File + writeFile *os.File +} + +func newStdoutCapture() *StdoutCapture { + s := &StdoutCapture{oldStdout: os.Stdout} + + AfterEach(func() { + s.done() + }) + + return s +} + +func (s *StdoutCapture) start() { + var err error + + s.readFile, s.writeFile, err = os.Pipe() + Expect(err).To(Succeed()) + + os.Stdout = s.writeFile +} + +func (s *StdoutCapture) read() string { + s.writeFile.Close() + os.Stdout = s.oldStdout + + var buf bytes.Buffer + _, err := io.Copy(&buf, s.readFile) + Expect(err).To(Succeed()) + + return buf.String() +} + +func (s *StdoutCapture) done() { + os.Stdout = s.oldStdout +} diff --git a/pkg/reporter/tracker_test.go b/pkg/reporter/tracker_test.go new file mode 100644 index 00000000..b7d5e5a5 --- /dev/null +++ b/pkg/reporter/tracker_test.go @@ -0,0 +1,56 @@ +/* +SPDX-License-Identifier: Apache-2.0 + +Copyright Contributors to the Submariner project. + +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 reporter_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/submariner-io/admiral/pkg/reporter" +) + +var _ = Describe("Tracker", func() { + stdoutCapture := newStdoutCapture() + + var tracker *reporter.Tracker + + BeforeEach(func() { + tracker = reporter.NewTracker(reporter.Stdout()) + }) + + It("report and track failures and warnings", func() { + stdoutCapture.start() + tracker.Start("Start message") + Expect(stdoutCapture.read()).To(ContainSubstring("Start message")) + + stdoutCapture.start() + tracker.Warning("Warning message") + Expect(stdoutCapture.read()).To(ContainSubstring("Warning message")) + + stdoutCapture.start() + tracker.Failure("Failure message") + Expect(stdoutCapture.read()).To(ContainSubstring("Failure message")) + + Expect(tracker.HasWarnings()).To(BeTrue()) + Expect(tracker.HasFailures()).To(BeTrue()) + + tracker.Start("Start again") + Expect(tracker.HasWarnings()).To(BeFalse()) + Expect(tracker.HasFailures()).To(BeFalse()) + }) +}) diff --git a/pkg/resource/errors.go b/pkg/resource/errors.go index d7f3ea47..58048f1d 100644 --- a/pkg/resource/errors.go +++ b/pkg/resource/errors.go @@ -38,7 +38,9 @@ func IsNotFoundErr(err error) bool { } for _, err := range errGDF.Groups { - return IsNotFoundErr(err) + if IsNotFoundErr(err) { + return true + } } return false diff --git a/pkg/resource/errors_test.go b/pkg/resource/errors_test.go index f4e788a0..49f20e48 100644 --- a/pkg/resource/errors_test.go +++ b/pkg/resource/errors_test.go @@ -70,7 +70,7 @@ var _ = Describe("IsNotFoundErr", func() { }) }) -var _ = Describe("IsMissingNamespaceErr and ExtractMissingNamespaceFromErr", func() { +var _ = Describe("IsMissingNamespaceErr", func() { When("the error isn't NotFound", func() { It("should return false", func() { Expect(resource.IsMissingNamespaceErr(apierrors.NewBadRequest(""))).To(BeFalse()) @@ -78,12 +78,10 @@ var _ = Describe("IsMissingNamespaceErr and ExtractMissingNamespaceFromErr", fun }) When("the error details specify a namespace", func() { - It("should return true, and the name should be retrievable", func() { - err := apierrors.NewNotFound(schema.GroupResource{ + It("should return true", func() { + Expect(resource.IsMissingNamespaceErr(apierrors.NewNotFound(schema.GroupResource{ Resource: "namespaces", - }, "missing-ns") - Expect(resource.IsMissingNamespaceErr(err)).To(BeTrue()) - Expect(resource.ExtractMissingNamespaceFromErr(err)).To(Equal("missing-ns")) + }, "missing-ns"))).To(BeTrue()) }) }) @@ -96,6 +94,22 @@ var _ = Describe("IsMissingNamespaceErr and ExtractMissingNamespaceFromErr", fun }) }) +var _ = Describe("ExtractMissingNamespaceFromErr", func() { + When("the error isn't NotFound", func() { + It("should return empty", func() { + Expect(resource.ExtractMissingNamespaceFromErr(apierrors.NewBadRequest(""))).To(BeEmpty()) + }) + }) + + When("the error details specify a namespace", func() { + It("should return the namespace", func() { + Expect(resource.ExtractMissingNamespaceFromErr(apierrors.NewNotFound(schema.GroupResource{ + Resource: "namespaces", + }, "missing-ns"))).To(Equal("missing-ns")) + }) + }) +}) + var _ = Describe("IsTransientErr", func() { When("the error is ServerTimeout", func() { It("should return true", func() { diff --git a/pkg/resource/interface_test.go b/pkg/resource/interface_test.go index f81a5894..6e9cf4fc 100644 --- a/pkg/resource/interface_test.go +++ b/pkg/resource/interface_test.go @@ -25,6 +25,7 @@ import ( . "github.com/onsi/gomega" "github.com/submariner-io/admiral/pkg/resource" "github.com/submariner-io/admiral/pkg/syncer/test" + "github.com/submariner-io/admiral/pkg/util" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -54,6 +55,9 @@ var _ = Describe("Interface", func() { Spec: appsv1.DaemonSetSpec{ MinReadySeconds: 3, }, + }, func(ds *appsv1.DaemonSet) *appsv1.DaemonSet { + ds.Status.NumberReady = 2 + return ds }) }) @@ -68,6 +72,9 @@ var _ = Describe("Interface", func() { Spec: appsv1.DeploymentSpec{ MinReadySeconds: 3, }, + }, func(d *appsv1.Deployment) *appsv1.Deployment { + d.Status.Replicas = 2 + return d }) }) @@ -78,6 +85,9 @@ var _ = Describe("Interface", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test", }, + }, func(ns *corev1.Namespace) *corev1.Namespace { + ns.Status.Phase = corev1.NamespaceActive + return ns }) }) @@ -92,6 +102,9 @@ var _ = Describe("Interface", func() { Spec: corev1.PodSpec{ Hostname: "my-host", }, + }, func(pod *corev1.Pod) *corev1.Pod { + pod.Status.Phase = corev1.PodPending + return pod }) }) @@ -106,6 +119,12 @@ var _ = Describe("Interface", func() { Spec: corev1.ServiceSpec{ Type: corev1.ServiceTypeClusterIP, }, + }, func(s *corev1.Service) *corev1.Service { + _ = meta.SetStatusCondition(&s.Status.Conditions, metav1.Condition{ + Type: "type", + Status: metav1.ConditionTrue, + }) + return s }) }) @@ -117,7 +136,7 @@ var _ = Describe("Interface", func() { Name: "test", Namespace: test.LocalNamespace, }, - }) + }, nil) }) Context("ForClusterRole", func() { @@ -127,7 +146,7 @@ var _ = Describe("Interface", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test", }, - }) + }, nil) }) Context("ForClusterRoleBinding", func() { @@ -137,7 +156,7 @@ var _ = Describe("Interface", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test", }, - }) + }, nil) }) Context("ForRole", func() { @@ -148,7 +167,7 @@ var _ = Describe("Interface", func() { Name: "test", Namespace: test.LocalNamespace, }, - }) + }, nil) }) Context("ForRoleBinding", func() { @@ -159,7 +178,7 @@ var _ = Describe("Interface", func() { Name: "test", Namespace: test.LocalNamespace, }, - }) + }, nil) }) Context("ForConfigMap", func() { @@ -171,7 +190,7 @@ var _ = Describe("Interface", func() { Namespace: test.LocalNamespace, }, Data: map[string]string{"one": "two"}, - }) + }, nil) }) Context("ForListableControllerClient", func() { @@ -190,6 +209,9 @@ var _ = Describe("Interface", func() { Spec: corev1.PodSpec{ Hostname: "my-host", }, + }, func(pod *corev1.Pod) *corev1.Pod { + pod.Status.Phase = corev1.PodPending + return pod }) }) @@ -213,11 +235,14 @@ var _ = Describe("Interface", func() { Spec: corev1.PodSpec{ Hostname: "my-host", }, - })) + }), func(u *unstructured.Unstructured) *unstructured.Unstructured { + util.SetNestedField(u.Object, "node", util.StatusField, "nodeName") + return u + }) }) }) -func testInterfaceFuncs[T runtime.Object](newInterface func() resource.Interface[T], initialObj T) { +func testInterfaceFuncs[T runtime.Object](newInterface func() resource.Interface[T], initialObj T, updateStatus func(T) T) { Specify("verify functions", func() { sanitize := func(o T) T { m := resource.MustToMeta(o) @@ -259,6 +284,16 @@ func testInterfaceFuncs[T runtime.Object](newInterface func() resource.Interface Expect(err).To(Succeed()) Expect(sanitize(obj)).To(Equal(actual)) + if updateStatus != nil { + actual = updateStatus(actual) + obj, err = i.UpdateStatus(context.Background(), actual, metav1.UpdateOptions{}) + Expect(err).To(Succeed()) + Expect(sanitize(obj)).To(Equal(actual)) + } else { + _, err = i.UpdateStatus(context.Background(), actual, metav1.UpdateOptions{}) + Expect(err).To(HaveOccurred()) + } + // List list, err := i.List(context.Background(), metav1.ListOptions{}) Expect(err).To(Succeed()) diff --git a/pkg/resource/resource_test.go b/pkg/resource/resource_test.go index 68243fcc..c5d26c1c 100644 --- a/pkg/resource/resource_test.go +++ b/pkg/resource/resource_test.go @@ -24,6 +24,8 @@ import ( "github.com/submariner-io/admiral/pkg/resource" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/cache" "k8s.io/utils/ptr" ) @@ -114,3 +116,177 @@ var _ = Describe("TrimManagedFields", func() { }) }) }) + +var _ = Describe("ToUnstructured", func() { + Context("with an Unstructured instance", func() { + It("should return a copy", func() { + obj := newUnstructuredPod() + + u, err := resource.ToUnstructured(obj) + Expect(err).To(Succeed()) + Expect(u).To(Equal(obj)) + + _ = unstructured.SetNestedField(u.Object, "host", "spec", "hostName") + Expect(u).ToNot(Equal(obj)) + }) + }) + + Context("with a Pod instance", func() { + It("should return a correct Unstructured instance", func() { + pod := newPod() + + u, err := resource.ToUnstructured(pod) + Expect(err).To(Succeed()) + Expect(u.GetName()).To(Equal(pod.Name)) + Expect(u.GetNamespace()).To(Equal(pod.Namespace)) + Expect(u.GetLabels()).To(Equal(pod.Labels)) + + s, _, _ := unstructured.NestedString(u.Object, "spec", "nodeName") + Expect(s).To(Equal(pod.Spec.NodeName)) + }) + }) +}) + +var _ = Describe("ToUnstructuredUsingScheme", func() { + When("the Pod kind is not in the scheme", func() { + It("should return an error", func() { + _, err := resource.ToUnstructuredUsingScheme(&corev1.Pod{}, runtime.NewScheme()) + Expect(err).To(HaveOccurred()) + }) + }) +}) + +var _ = Describe("MustToUnstructuredUsingScheme", func() { + When("the Pod kind is not in the scheme", func() { + It("should panic", func() { + Expect(func() { + _ = resource.MustToUnstructuredUsingScheme(&corev1.Pod{}, runtime.NewScheme()) + }).To(Panic()) + }) + }) +}) + +var _ = Describe("MustToUnstructuredUsingDefaultConverter", func() { + Context("with an Unstructured instance", func() { + It("should return a copy", func() { + obj := newUnstructuredPod() + + u := resource.MustToUnstructuredUsingDefaultConverter(newUnstructuredPodWithKind()) + Expect(u).To(Equal(obj)) + + _ = unstructured.SetNestedField(u.Object, "host", "spec", "hostName") + Expect(u).ToNot(Equal(obj)) + }) + }) + + Context("with a Pod instance", func() { + It("should return a correct Unstructured instance", func() { + pod := newPod() + + u := resource.MustToUnstructuredUsingDefaultConverter(pod) + Expect(u.GetName()).To(Equal(pod.Name)) + Expect(u.GetNamespace()).To(Equal(pod.Namespace)) + Expect(u.GetLabels()).To(Equal(pod.Labels)) + + s, _, _ := unstructured.NestedString(u.Object, "spec", "nodeName") + Expect(s).To(Equal(pod.Spec.NodeName)) + }) + }) +}) + +var _ = Describe("MustFromUnstructured", func() { + It("should return a correct Unstructured instance", func() { + u := newUnstructuredPodWithKind() + pod := resource.MustFromUnstructured(u, &corev1.Pod{}) + + Expect(pod.Name).To(Equal(u.GetName())) + Expect(pod.Namespace).To(Equal(u.GetNamespace())) + Expect(pod.Labels).To(Equal(u.GetLabels())) + + s, _, _ := unstructured.NestedString(u.Object, "spec", "nodeName") + Expect(s).To(Equal(pod.Spec.NodeName)) + }) + + When("the Kind field is missing", func() { + It("should panic", func() { + Expect(func() { + _ = resource.MustFromUnstructured(newUnstructuredPod(), &corev1.Pod{}) + }).To(Panic()) + }) + }) +}) + +var _ = Describe("MustFromUnstructuredUsingScheme", func() { + When("the Pod kind is not in the scheme", func() { + It("should panic", func() { + Expect(func() { + _ = resource.MustFromUnstructuredUsingScheme(newUnstructuredPodWithKind(), &corev1.Pod{}, runtime.NewScheme()) + }).To(Panic()) + }) + }) +}) + +var _ = Describe("MustToMeta", func() { + It("should return the meta Object", func() { + pod := newPod() + Expect(resource.MustToMeta(pod).GetName()).To(Equal(pod.Name)) + + u := newUnstructuredPod() + Expect(resource.MustToMeta(u).GetName()).To(Equal(u.GetName())) + }) + + When("the instance isn't a meta Object", func() { + It("should panic", func() { + Expect(func() { + _ = resource.MustToMeta(&corev1.PodSpec{}) + }).To(Panic()) + }) + }) +}) + +var _ = Describe("ToJSON", func() { + It("should return the JSON string", func() { + pod := newPod() + json := resource.ToJSON(pod) + + Expect(json).To(ContainSubstring("%q: %q", "name", pod.Name)) + Expect(json).To(ContainSubstring("%q: %q", "namespace", pod.Namespace)) + Expect(json).To(ContainSubstring("%q: %q", "nodeName", pod.Spec.NodeName)) + }) +}) + +func newUnstructuredPod() *unstructured.Unstructured { + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "name": "test-pod", + "namespace": "ns", + "labels": map[string]interface{}{"app": "test"}, + }, + "spec": map[string]interface{}{ + "nodeName": "node", + }, + }, + } +} + +func newUnstructuredPodWithKind() *unstructured.Unstructured { + u := newUnstructuredPod() + u.SetKind("Pod") + u.SetAPIVersion("v1") + + return u +} + +func newPod() *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "ns", + Labels: map[string]string{"app": "test"}, + }, + Spec: corev1.PodSpec{ + NodeName: "node", + }, + } +} diff --git a/pkg/resource/util.go b/pkg/resource/util.go index c6cc353c..5bf36f21 100644 --- a/pkg/resource/util.go +++ b/pkg/resource/util.go @@ -72,9 +72,7 @@ func MustToUnstructuredUsingDefaultConverter(from runtime.Object) *unstructured. u = f.DeepCopy() default: m, err := runtime.DefaultUnstructuredConverter.ToUnstructured(from) - if err != nil { - panic(err) - } + utilruntime.Must(err) u = &unstructured.Unstructured{Object: m} } @@ -101,9 +99,7 @@ func MustFromUnstructuredUsingScheme[T runtime.Object](u *unstructured.Unstructu func MustToMeta(obj interface{}) metav1.Object { objMeta, err := meta.Accessor(obj) - if err != nil { - panic(err) - } + utilruntime.Must(err) return objMeta } diff --git a/pkg/util/helper_test.go b/pkg/util/helper_test.go index 4206238c..7f1f47f1 100644 --- a/pkg/util/helper_test.go +++ b/pkg/util/helper_test.go @@ -19,11 +19,18 @@ limitations under the License. package util_test import ( + "crypto/x509" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/submariner-io/admiral/pkg/resource" + "github.com/submariner-io/admiral/pkg/syncer/test" "github.com/submariner-io/admiral/pkg/util" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" ) var _ = Describe("DeeplyEmpty", func() { @@ -76,3 +83,129 @@ var _ = Describe("DeeplyEmpty", func() { util.StatusField).(map[string]interface{}))).To(BeTrue()) }) }) + +var _ = Describe("FindGroupVersionResource", func() { + Specify("should return the GVR if existing", func() { + restMapper := test.GetRESTMapperFor(&corev1.Pod{}) + + gvr, err := util.FindGroupVersionResource(resource.MustToUnstructured(&corev1.Pod{}), restMapper) + Expect(err).To(Succeed()) + Expect(*gvr).To(Equal(corev1.SchemeGroupVersion.WithResource("pods"))) + }) + + Specify("should return an error if non-existent", func() { + restMapper := test.GetRESTMapperFor(&corev1.Pod{}) + + _, err := util.FindGroupVersionResource(resource.MustToUnstructured(&corev1.Service{}), restMapper) + Expect(err).To(HaveOccurred()) + }) +}) + +var _ = Describe("ToUnstructuredResource", func() { + Specify("should return the Unstructured obj and GVR on success", func() { + restMapper := test.GetRESTMapperFor(&corev1.Pod{}) + + obj, gvr, err := util.ToUnstructuredResource(&corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + }, + Spec: corev1.PodSpec{ + NodeName: "node", + }, + }, restMapper) + + Expect(err).To(Succeed()) + Expect(*gvr).To(Equal(corev1.SchemeGroupVersion.WithResource("pods"))) + Expect(obj.GetName()).To(Equal("test-pod")) + + spec := &corev1.PodSpec{} + _ = runtime.DefaultUnstructuredConverter.FromUnstructured(util.GetSpec(obj).(map[string]interface{}), spec) + Expect(spec.NodeName).To(Equal("node")) + }) + + Specify("should return an error if GVR non-existent", func() { + restMapper := test.GetRESTMapperFor(&corev1.Service{}) + + _, _, err := util.ToUnstructuredResource(&corev1.Pod{}, restMapper) + Expect(err).To(HaveOccurred()) + }) +}) + +var _ = Describe("AddCertificateErrorHandler", func() { + unknownAuthorityError := x509.UnknownAuthorityError{Cert: &x509.Certificate{ + Raw: []byte{1, 2, 3}, + }} + + certificateInvalidError := x509.CertificateInvalidError{Cert: &x509.Certificate{ + Raw: []byte{4, 5, 6}, + }} + + var ( + errorLogged chan error + fatalLogged chan error + ) + + BeforeEach(func() { + errorLogged = make(chan error, 20) + fatalLogged = make(chan error, 20) + + util.ErrorHook = func(err error, _ string, _ ...interface{}) { + errorLogged <- err + } + + util.FatalHook = func(err error, _ string, _ ...interface{}) { + fatalLogged <- err + } + + savedErrorHandlers := utilruntime.ErrorHandlers + DeferCleanup(func() { + utilruntime.ErrorHandlers = savedErrorHandlers + }) + }) + + testCertificateErrorHandler := func(fatal bool, logged chan error) { + util.AddCertificateErrorHandler(fatal) + + utilruntime.HandleError(unknownAuthorityError) + Expect(logged).To(Receive()) + + utilruntime.HandleError(unknownAuthorityError) + Expect(logged).ToNot(Receive()) + + utilruntime.HandleError(certificateInvalidError) + Expect(logged).To(Receive()) + + utilruntime.HandleError(certificateInvalidError) + Expect(logged).ToNot(Receive()) + } + + Context("with non-fatal", func() { + It("should log an error message on first certificate error", func() { + testCertificateErrorHandler(false, errorLogged) + }) + }) + + Context("with fatal", func() { + It("should log a fatal message on first certificate error", func() { + testCertificateErrorHandler(true, fatalLogged) + }) + }) +}) + +var _ = Describe("GetMetadata", func() { + Specify("should return the metadata map if existing", func() { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Labels: map[string]string{"foo": "bar"}, + }, + } + + objMeta := util.GetMetadata(resource.MustToUnstructured(pod)) + Expect(objMeta).ToNot(BeNil()) + Expect(objMeta).To(HaveKeyWithValue("name", pod.Name)) + Expect(objMeta).To(HaveKeyWithValue("labels", map[string]interface{}{"foo": "bar"})) + + Expect(util.GetMetadata(&unstructured.Unstructured{})).To(BeEmpty()) + }) +}) diff --git a/pkg/util/helpers.go b/pkg/util/helpers.go index 37f053ad..793000f8 100644 --- a/pkg/util/helpers.go +++ b/pkg/util/helpers.go @@ -21,7 +21,6 @@ package util import ( "context" "crypto/x509" - "fmt" "sync/atomic" "github.com/pkg/errors" @@ -99,9 +98,7 @@ func GetSpec(obj *unstructured.Unstructured) interface{} { func GetNestedField(obj *unstructured.Unstructured, fields ...string) interface{} { nested, _, err := unstructured.NestedFieldNoCopy(obj.Object, fields...) - if err != nil { - panic(fmt.Sprintf("Error retrieving %v field for %#v: %v", fields, obj, err)) - } + utilruntime.Must(errors.Wrapf(err, "error retrieving %v field for %#v", fields, obj)) return nested } @@ -109,9 +106,7 @@ func GetNestedField(obj *unstructured.Unstructured, fields ...string) interface{ func SetNestedField(to map[string]interface{}, value interface{}, fields ...string) { if value != nil { err := unstructured.SetNestedField(to, value, fields...) - if err != nil { - panic(fmt.Sprintf("Error setting value (%v) for nested field %v in object %v: %v", value, fields, to, err)) - } + utilruntime.Must(errors.Wrapf(err, "error setting value (%v) for nested field %v in object %v", value, fields, to)) } } @@ -124,14 +119,10 @@ func CopyImmutableMetadata(from, to *unstructured.Unstructured) *unstructured.Un fromMetadata := value.(map[string]interface{}) err := unstructured.SetNestedStringMap(fromMetadata, to.GetLabels(), LabelsField) - if err != nil { - panic(err) - } + utilruntime.Must(err) err = unstructured.SetNestedStringMap(fromMetadata, to.GetAnnotations(), AnnotationsField) - if err != nil { - panic(err) - } + utilruntime.Must(err) SetNestedField(to.Object, fromMetadata, MetadataField) @@ -153,10 +144,15 @@ func DeeplyEmpty(m map[string]interface{}) bool { return true } +var ( + ErrorHook = logger.Errorf + FatalHook = logger.FatalfOnError +) + func AddCertificateErrorHandler(fatal bool) { - logCertificateError := logger.Errorf + logCertificateError := ErrorHook if fatal { - logCertificateError = logger.FatalfOnError + logCertificateError = FatalHook } utilruntime.ErrorHandlers = append(utilruntime.ErrorHandlers, diff --git a/sonar-project.properties b/sonar-project.properties index 4a00ae73..bf8bb19e 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -2,7 +2,7 @@ sonar.projectKey=submariner-io_admiral sonar.projectName=admiral sonar.organization=submariner-io sonar.sources=. -sonar.exclusions=**/vendor/**,**/*_test.go,**/test/**,**/mock/**,**/fake/** +sonar.exclusions=**/vendor/**,**/*_test.go,**/test/**,**/mock/**,**/fake/**,**/gomega/** sonar.tests=. sonar.test.inclusions=**/*_test.go,**/test/**,**/mock/**,**/fake/** sonar.test.exclusions=**/vendor/**