From d01cb44dc812f8bb6a2f9613e8b77afb0de8932b Mon Sep 17 00:00:00 2001 From: kahirokunn Date: Mon, 19 May 2025 17:53:41 +0900 Subject: [PATCH 1/2] =?UTF-8?q?=E2=9C=A8clusterctl:=20validate=20provider?= =?UTF-8?q?=20metadata=20and=20improve=20debug=20logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds stricter validation and richer diagnostics while reading provider metadata: * Introduce validateMetadata() to verify: * apiVersion matches clusterctl’s expected group/version * kind is “Metadata” * releaseSeries is not empty * Return clear, wrapped errors when decoding or validation fails. These changes help surface mis-shaped metadata early and make failures easier to troubleshoot. Signed-off-by: kahirokunn --- .../client/repository/metadata_client.go | 34 +++++++- .../client/repository/metadata_client_test.go | 82 +++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) diff --git a/cmd/clusterctl/client/repository/metadata_client.go b/cmd/clusterctl/client/repository/metadata_client.go index 84a7897b575a..254125dde89c 100644 --- a/cmd/clusterctl/client/repository/metadata_client.go +++ b/cmd/clusterctl/client/repository/metadata_client.go @@ -92,7 +92,39 @@ func (f *metadataClient) Get(ctx context.Context) (*clusterctlv1.Metadata, error return nil, errors.Wrapf(err, "error decoding %q for provider %q", metadataFile, f.provider.ManifestLabel()) } - //TODO: consider if to add metadata validation (TBD) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse current version %q for provider %q", f.version, f.provider.ManifestLabel()) + } + if err := validateMetadata(obj, f.provider.ManifestLabel()); err != nil { + return nil, err + } return obj, nil } + +// validateMetadata validates the metadata object structure. +// +// It checks if: +// 1. The metadata has the correct apiVersion and kind. +// 2. The metadata has at least one release series. +// +// Note: Version matching against releaseSeries is done later in `installer.go`. +func validateMetadata(metadata *clusterctlv1.Metadata, providerLabel string) error { + // Check if metadata has the correct apiVersion and kind + if metadata.APIVersion != clusterctlv1.GroupVersion.String() { + return errors.Errorf("invalid provider metadata: unexpected apiVersion %q for provider %s (expected %q). Full metadata.yaml content is available in logs with --v=5", + metadata.APIVersion, providerLabel, clusterctlv1.GroupVersion.String()) + } + + if metadata.Kind != "Metadata" { + return errors.Errorf("invalid provider metadata: unexpected kind %q for provider %s (expected \"Metadata\"). Full metadata.yaml content is available in logs with --v=5", + metadata.Kind, providerLabel) + } + + // Check if metadata has at least one release series + if len(metadata.ReleaseSeries) == 0 { + return errors.Errorf("invalid provider metadata: releaseSeries is empty in metadata.yaml for provider %s. Full metadata.yaml content is available in logs with --v=5", providerLabel) + } + + return nil +} diff --git a/cmd/clusterctl/client/repository/metadata_client_test.go b/cmd/clusterctl/client/repository/metadata_client_test.go index e194784e47f6..b3d20e28d0d1 100644 --- a/cmd/clusterctl/client/repository/metadata_client_test.go +++ b/cmd/clusterctl/client/repository/metadata_client_test.go @@ -135,3 +135,85 @@ func Test_metadataClient_Get(t *testing.T) { }) } } + +func Test_validateMetadata(t *testing.T) { + tests := []struct { + name string + metadata *clusterctlv1.Metadata + providerLabel string + wantErr bool + errMessage string + }{ + { + name: "valid metadata", + metadata: &clusterctlv1.Metadata{ + TypeMeta: metav1.TypeMeta{ + APIVersion: clusterctlv1.GroupVersion.String(), + Kind: "Metadata", + }, + ReleaseSeries: []clusterctlv1.ReleaseSeries{ + {Major: 1, Minor: 0, Contract: "v1beta1"}, + }, + }, + providerLabel: "infra-test", + wantErr: false, + }, + { + name: "invalid apiVersion", + metadata: &clusterctlv1.Metadata{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "wrong.group/v1", + Kind: "Metadata", + }, + ReleaseSeries: []clusterctlv1.ReleaseSeries{ + {Major: 1, Minor: 0, Contract: "v1beta1"}, + }, + }, + providerLabel: "infra-test", + wantErr: true, + errMessage: "invalid provider metadata: unexpected apiVersion \"wrong.group/v1\" for provider infra-test (expected \"clusterctl.cluster.x-k8s.io/v1alpha3\")", + }, + { + name: "invalid kind", + metadata: &clusterctlv1.Metadata{ + TypeMeta: metav1.TypeMeta{ + APIVersion: clusterctlv1.GroupVersion.String(), + Kind: "WrongKind", + }, + ReleaseSeries: []clusterctlv1.ReleaseSeries{ + {Major: 1, Minor: 0, Contract: "v1beta1"}, + }, + }, + providerLabel: "infra-test", + wantErr: true, + errMessage: "invalid provider metadata: unexpected kind \"WrongKind\" for provider infra-test (expected \"Metadata\")", + }, + { + name: "empty releaseSeries", + metadata: &clusterctlv1.Metadata{ + TypeMeta: metav1.TypeMeta{ + APIVersion: clusterctlv1.GroupVersion.String(), + Kind: "Metadata", + }, + ReleaseSeries: []clusterctlv1.ReleaseSeries{}, + }, + providerLabel: "infra-test", + wantErr: true, + errMessage: "invalid provider metadata: releaseSeries is empty in metadata.yaml for provider infra-test", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + err := validateMetadata(tt.metadata, tt.providerLabel) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.errMessage)) + return + } + g.Expect(err).ToNot(HaveOccurred()) + }) + } +} From b5e1d311ecbd90682acfcf0b003c234ac30e3114 Mon Sep 17 00:00:00 2001 From: kahirokunn Date: Thu, 29 May 2025 09:27:54 +0900 Subject: [PATCH 2/2] Update cmd/clusterctl/client/repository/metadata_client.go Co-authored-by: Fabrizio Pandini Signed-off-by: kahirokunn --- cmd/clusterctl/client/repository/metadata_client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/clusterctl/client/repository/metadata_client.go b/cmd/clusterctl/client/repository/metadata_client.go index 254125dde89c..695cf8d6e5e3 100644 --- a/cmd/clusterctl/client/repository/metadata_client.go +++ b/cmd/clusterctl/client/repository/metadata_client.go @@ -112,18 +112,18 @@ func (f *metadataClient) Get(ctx context.Context) (*clusterctlv1.Metadata, error func validateMetadata(metadata *clusterctlv1.Metadata, providerLabel string) error { // Check if metadata has the correct apiVersion and kind if metadata.APIVersion != clusterctlv1.GroupVersion.String() { - return errors.Errorf("invalid provider metadata: unexpected apiVersion %q for provider %s (expected %q). Full metadata.yaml content is available in logs with --v=5", + return errors.Errorf("invalid provider metadata: unexpected apiVersion %q for provider %s (expected %q)", metadata.APIVersion, providerLabel, clusterctlv1.GroupVersion.String()) } if metadata.Kind != "Metadata" { - return errors.Errorf("invalid provider metadata: unexpected kind %q for provider %s (expected \"Metadata\"). Full metadata.yaml content is available in logs with --v=5", + return errors.Errorf("invalid provider metadata: unexpected kind %q for provider %s (expected \"Metadata\")", metadata.Kind, providerLabel) } // Check if metadata has at least one release series if len(metadata.ReleaseSeries) == 0 { - return errors.Errorf("invalid provider metadata: releaseSeries is empty in metadata.yaml for provider %s. Full metadata.yaml content is available in logs with --v=5", providerLabel) + return errors.Errorf("invalid provider metadata: releaseSeries is empty in metadata.yaml for provider %s", providerLabel) } return nil