From e2318bcc4586fda4eb16520947b8241a7c43bfb7 Mon Sep 17 00:00:00 2001 From: nueavv Date: Thu, 6 Mar 2025 16:24:24 +0900 Subject: [PATCH] fix(taint deletion): handle nil values to prevent panic --- go.mod | 2 + pkg/cloud/converters/eks.go | 4 +- pkg/cloud/converters/eks_test.go | 191 +++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 2 deletions(-) create mode 100644 pkg/cloud/converters/eks_test.go diff --git a/go.mod b/go.mod index ee962015b5..79f0f73b59 100644 --- a/go.mod +++ b/go.mod @@ -33,6 +33,7 @@ require ( github.com/sirupsen/logrus v1.9.3 github.com/spf13/cobra v1.8.1 github.com/spf13/pflag v1.0.6-0.20210604193023-d5e0c0615ace + github.com/stretchr/testify v1.9.0 github.com/zgalor/weberr v0.8.2 golang.org/x/crypto v0.31.0 golang.org/x/text v0.21.0 @@ -176,6 +177,7 @@ require ( github.com/opencontainers/image-spec v1.1.0-rc5 // indirect github.com/pelletier/go-toml v1.9.5 // indirect github.com/pelletier/go-toml/v2 v2.2.2 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_model v0.6.1 // indirect github.com/prometheus/common v0.55.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect diff --git a/pkg/cloud/converters/eks.go b/pkg/cloud/converters/eks.go index d9985f4693..5e2a9b5868 100644 --- a/pkg/cloud/converters/eks.go +++ b/pkg/cloud/converters/eks.go @@ -111,8 +111,8 @@ func TaintsFromSDK(taints []*eks.Taint) (expinfrav1.Taints, error) { } converted = append(converted, expinfrav1.Taint{ Effect: convertedEffect, - Key: *taint.Key, - Value: *taint.Value, + Key: aws.StringValue(taint.Key), + Value: aws.StringValue(taint.Value), }) } diff --git a/pkg/cloud/converters/eks_test.go b/pkg/cloud/converters/eks_test.go new file mode 100644 index 0000000000..71373cb684 --- /dev/null +++ b/pkg/cloud/converters/eks_test.go @@ -0,0 +1,191 @@ +/* +Copyright 2024 The Kubernetes Authors. + +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 converters + +import ( + "testing" + + "github.com/aws/aws-sdk-go/service/eks" + "github.com/stretchr/testify/assert" + "k8s.io/utils/ptr" + + expinfrav1 "sigs.k8s.io/cluster-api-provider-aws/v2/exp/api/v1beta2" +) + +func TestTaintsFromSDK(t *testing.T) { + tests := []struct { + name string + input []*eks.Taint + expected expinfrav1.Taints + expectErr bool + }{ + { + name: "Single taint conversion", + input: []*eks.Taint{ + { + Key: ptr.To("key1"), + Value: ptr.To("value1"), + Effect: ptr.To(eks.TaintEffectNoSchedule), + }, + }, + expected: expinfrav1.Taints{ + { + Key: "key1", + Value: "value1", + Effect: expinfrav1.TaintEffectNoSchedule, + }, + }, + expectErr: false, + }, + { + name: "Multiple taints conversion", + input: []*eks.Taint{ + { + Key: ptr.To("key1"), + Value: ptr.To("value1"), + Effect: ptr.To(eks.TaintEffectNoExecute), + }, + { + Key: ptr.To("key2"), + Value: ptr.To("value2"), + Effect: ptr.To(eks.TaintEffectPreferNoSchedule), + }, + }, + expected: expinfrav1.Taints{ + { + Key: "key1", + Value: "value1", + Effect: expinfrav1.TaintEffectNoExecute, + }, + { + Key: "key2", + Value: "value2", + Effect: expinfrav1.TaintEffectPreferNoSchedule, + }, + }, + expectErr: false, + }, + { + name: "Unknown taint effect", + input: []*eks.Taint{ + { + Key: ptr.To("key1"), + Value: ptr.To("value1"), + Effect: ptr.To("UnknownEffect"), + }, + }, + expected: nil, + expectErr: true, + }, + { + name: "Nil taint value", + input: []*eks.Taint{ + { + Key: ptr.To("key1"), + Value: nil, + Effect: ptr.To(eks.TaintEffectNoSchedule), + }, + }, + expected: expinfrav1.Taints{ + { + Key: "key1", + Value: "", + Effect: expinfrav1.TaintEffectNoSchedule, + }, + }, + expectErr: false, + }, + { + name: "Empty input slice", + input: []*eks.Taint{}, + expected: expinfrav1.Taints{}, + expectErr: false, + }, + { + name: "Nil input", + input: nil, + expected: expinfrav1.Taints{}, + expectErr: false, + }, + { + name: "Mixed valid and invalid taints", + input: []*eks.Taint{ + { + Key: ptr.To("key1"), + Value: ptr.To("value1"), + Effect: ptr.To(eks.TaintEffectNoExecute), + }, + { + Key: ptr.To("key2"), + Value: ptr.To("value2"), + Effect: ptr.To("InvalidEffect"), + }, + }, + expected: nil, + expectErr: true, + }, + { + name: "Empty key", + input: []*eks.Taint{ + { + Key: ptr.To(""), + Value: ptr.To("value1"), + Effect: ptr.To(eks.TaintEffectNoSchedule), + }, + }, + expected: expinfrav1.Taints{ + { + Key: "", + Value: "value1", + Effect: expinfrav1.TaintEffectNoSchedule, + }, + }, + expectErr: false, + }, + { + name: "Empty value", + input: []*eks.Taint{ + { + Key: ptr.To("key1"), + Value: ptr.To(""), + Effect: ptr.To(eks.TaintEffectNoSchedule), + }, + }, + expected: expinfrav1.Taints{ + { + Key: "key1", + Value: "", + Effect: expinfrav1.TaintEffectNoSchedule, + }, + }, + expectErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := TaintsFromSDK(tt.input) + if tt.expectErr { + assert.Error(t, err) + assert.Nil(t, result, "Expected result to be nil on error") + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, result, "Converted taints do not match expected") + } + }) + } +}