diff --git a/pkg/ctl/cmdutils/configfile_test.go b/pkg/ctl/cmdutils/configfile_test.go index d5185374a6..42964d7cba 100644 --- a/pkg/ctl/cmdutils/configfile_test.go +++ b/pkg/ctl/cmdutils/configfile_test.go @@ -140,7 +140,7 @@ var _ = Describe("cmdutils configfile", func() { l.flagsIncompatibleWithConfigFile.Delete("name") err := l.Load() - Expect(err).To(MatchError(ContainSubstring("error unmarshaling JSON: while decoding JSON: json: unknown field \"containerRuntime\""))) + Expect(err).To(MatchError(ContainSubstring("strict decoding error: unknown field \"managedNodeGroups[0].containerRuntime\""))) }) }) }) diff --git a/pkg/eks/api.go b/pkg/eks/api.go index 5e36056bce..e1bf87a95d 100644 --- a/pkg/eks/api.go +++ b/pkg/eks/api.go @@ -21,9 +21,9 @@ import ( "github.com/aws/aws-sdk-go-v2/service/sts" "github.com/kris-nova/logger" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/serializer" k8sclient "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/yaml" "github.com/weaveworks/eksctl/pkg/ami" api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5" @@ -217,18 +217,12 @@ func newAWSProvider(spec *api.ProviderConfig, configurationLoader AWSConfigurati return provider, nil } +// The default deserializer silently drops unknown and mis-cased keys. +var strictDeserializer = serializer.NewCodecFactory(scheme.Scheme, serializer.EnableStrict).UniversalDeserializer() + // ParseConfig parses data into a ClusterConfig func ParseConfig(data []byte) (*api.ClusterConfig, error) { - // strict mode is not available in runtime.Decode, so we use the parser - // directly; we don't store the resulting object, this is just the means - // of detecting any unknown keys - // NOTE: we must use sigs.k8s.io/yaml, as it behaves differently from - // github.com/ghodss/yaml, which didn't handle nested structs well - if err := yaml.UnmarshalStrict(data, &api.ClusterConfig{}); err != nil { - return nil, err - } - - obj, err := runtime.Decode(scheme.Codecs.UniversalDeserializer(), data) + obj, err := runtime.Decode(strictDeserializer, data) if err != nil { return nil, err } diff --git a/pkg/eks/api_test.go b/pkg/eks/api_test.go index 5d31b0f3e5..3d89570d12 100644 --- a/pkg/eks/api_test.go +++ b/pkg/eks/api_test.go @@ -215,19 +215,44 @@ var _ = Describe("eksctl API", func() { It("should reject unknown field in a YAML config", func() { _, err := eks.LoadConfigFromFile("testdata/bad-field-1.yaml") Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(HavePrefix(`loading config file "testdata/bad-field-1.yaml": error unmarshaling JSON: while decoding JSON: json: unknown field "zone"`)) + Expect(err.Error()).To(HavePrefix(`loading config file "testdata/bad-field-1.yaml": strict decoding error: unknown field "metadata.zone"`)) }) It("should reject unknown field in a YAML config", func() { _, err := eks.LoadConfigFromFile("testdata/bad-field-2.yaml") Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(HavePrefix(`loading config file "testdata/bad-field-2.yaml": error unmarshaling JSON: while decoding JSON: json: unknown field "bar"`)) + Expect(err.Error()).To(HavePrefix(`loading config file "testdata/bad-field-2.yaml": strict decoding error: unknown field "nodeGroups[0].iam.withAddonPolicies.bar"`)) }) It("should reject unknown field in a JSON config", func() { _, err := eks.LoadConfigFromFile("testdata/bad-field-1.json") Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(HavePrefix(`loading config file "testdata/bad-field-1.json": error unmarshaling JSON: while decoding JSON: json: unknown field "nodes"`)) + Expect(err.Error()).To(HavePrefix(`loading config file "testdata/bad-field-1.json": strict decoding error: unknown field "nodeGroups[0].nodes"`)) + }) + + It("should reject a duplicate key in a YAML config", func() { + _, err := eks.LoadConfigFromFile("testdata/duplicate-key-1.yaml") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(HavePrefix(`loading config file "testdata/duplicate-key-1.yaml": strict decoding error: yaml: unmarshal errors:`)) + Expect(err.Error()).To(ContainSubstring(`key "name" already set in map`)) + }) + + It("should report a type error before unknown fields", func() { + _, err := eks.LoadConfigFromFile("testdata/bad-type-2.yaml") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(HavePrefix(`loading config file "testdata/bad-type-2.yaml": json: cannot unmarshal bool into Go struct field`)) + }) + + It("should reject a top-level field with the wrong case", func() { + _, err := eks.LoadConfigFromFile("testdata/bad-field-case-1.yaml") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(HavePrefix(`loading config file "testdata/bad-field-case-1.yaml": strict decoding error: unknown field "NodeGroups"`)) + }) + + It("should reject a nested field with the wrong case", func() { + _, err := eks.LoadConfigFromFile("testdata/bad-field-case-2.yaml") + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(HavePrefix(`loading config file "testdata/bad-field-case-2.yaml": strict decoding error: unknown field "managedNodeGroups[0].capacityReservation.capacityReservationTarget.capacityReservationId"`)) }) It("should reject old API version", func() { diff --git a/pkg/eks/testdata/bad-field-1.json b/pkg/eks/testdata/bad-field-1.json index d717554e6b..c2882eb0dc 100644 --- a/pkg/eks/testdata/bad-field-1.json +++ b/pkg/eks/testdata/bad-field-1.json @@ -1,5 +1,5 @@ { - "apiVersion": "eksctl.io/v1alpha3", + "apiVersion": "eksctl.io/v1alpha5", "kind": "ClusterConfig", "metadata": { "name": "cluster-1", diff --git a/pkg/eks/testdata/bad-field-2.yaml b/pkg/eks/testdata/bad-field-2.yaml index 7f1279147a..7641fe8e8e 100644 --- a/pkg/eks/testdata/bad-field-2.yaml +++ b/pkg/eks/testdata/bad-field-2.yaml @@ -10,7 +10,8 @@ nodeGroups: - name: ng-1 instanceType: m5.large desiredCapacity: 10 - ssh: true + ssh: + allow: true iam: withAddonPolicies: foo: 1 diff --git a/pkg/eks/testdata/bad-field-case-1.yaml b/pkg/eks/testdata/bad-field-case-1.yaml new file mode 100644 index 0000000000..3b66b88d13 --- /dev/null +++ b/pkg/eks/testdata/bad-field-case-1.yaml @@ -0,0 +1,11 @@ +--- +apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig + +metadata: + name: cluster-1 + region: eu-north-1 + +NodeGroups: + - name: ng-1 + instanceType: m5.large diff --git a/pkg/eks/testdata/bad-field-case-2.yaml b/pkg/eks/testdata/bad-field-case-2.yaml new file mode 100644 index 0000000000..19bdd7433c --- /dev/null +++ b/pkg/eks/testdata/bad-field-case-2.yaml @@ -0,0 +1,13 @@ +--- +apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig + +metadata: + name: cluster-1 + region: eu-north-1 + +managedNodeGroups: + - name: mng-1 + capacityReservation: + capacityReservationTarget: + capacityReservationId: cr-0123456789abcdef0 diff --git a/pkg/eks/testdata/bad-type-2.yaml b/pkg/eks/testdata/bad-type-2.yaml new file mode 100644 index 0000000000..773dafe7a3 --- /dev/null +++ b/pkg/eks/testdata/bad-type-2.yaml @@ -0,0 +1,14 @@ +--- +apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig + +metadata: + name: cluster-1 + region: eu-north-1 + +nodeGroups: + - name: ng-1 + ssh: true + iam: + withAddonPolicies: + foo: 1 diff --git a/pkg/eks/testdata/duplicate-key-1.yaml b/pkg/eks/testdata/duplicate-key-1.yaml new file mode 100644 index 0000000000..6237d7ec1d --- /dev/null +++ b/pkg/eks/testdata/duplicate-key-1.yaml @@ -0,0 +1,8 @@ +--- +apiVersion: eksctl.io/v1alpha5 +kind: ClusterConfig + +metadata: + name: cluster-1 + name: cluster-2 + region: eu-north-1