PR feedback - consistently use sigs.k8s.io/yaml

gardener · Mar 29, 2023 · cfb2bcc · cfb2bcc
1 parent cadff5a
commit cfb2bcc
Show file tree

Hide file tree

Showing 8 changed files with 84 additions and 50 deletions.
diff --git a/go.mod b/go.mod
@@ -17,7 +17,6 @@ require (
 	github.com/spf13/pflag v1.0.5
 	github.com/spf13/viper v1.15.0
 	golang.org/x/crypto v0.7.0
-	gopkg.in/yaml.v3 v3.0.1
 	k8s.io/api v0.25.0
 	k8s.io/apimachinery v0.25.0
 	k8s.io/apiserver v0.25.0
@@ -120,6 +119,7 @@ require (
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 	istio.io/api v0.0.0-20221013011440-bc935762d2b9 // indirect
 	istio.io/client-go v1.15.3 // indirect
 	k8s.io/apiextensions-apiserver v0.25.0 // indirect

diff --git a/pkg/ac/access_restriction.go b/pkg/ac/access_restriction.go
@@ -20,23 +20,23 @@ import (
 // AccessRestriction is used to define an access restriction.
 type AccessRestriction struct {
 	// Key is the identifier of an access restriction
-	Key string `yaml:"key,omitempty" json:"key,omitempty"`
+	Key string `json:"key,omitempty"`
 	// NotifyIf controls which value the annotation must have for a notification to be sent
-	NotifyIf bool `yaml:"notifyIf,omitempty" json:"notifyIf,omitempty"`
+	NotifyIf bool `json:"notifyIf,omitempty"`
 	// Msg is the notification text that is sent
-	Msg string `yaml:"msg,omitempty" json:"msg,omitempty"`
+	Msg string `json:"msg,omitempty"`
 	// Options is a list of access restriction options
-	Options []AccessRestrictionOption `yaml:"options,omitempty" json:"options,omitempty"`
+	Options []AccessRestrictionOption `json:"options,omitempty"`
 }
 
 // AccessRestrictionOption is used to define an access restriction option.
 type AccessRestrictionOption struct {
 	// Key is the identifier of an access restriction option
-	Key string `yaml:"key,omitempty" json:"key,omitempty"`
+	Key string `json:"key,omitempty"`
 	// NotifyIf controls which value the annotation must have for a notification to be sent
-	NotifyIf bool `yaml:"notifyIf,omitempty" json:"notifyIf,omitempty"`
+	NotifyIf bool `json:"notifyIf,omitempty"`
 	// Msg is the notification text that is sent
-	Msg string `yaml:"msg,omitempty" json:"msg,omitempty"`
+	Msg string `json:"msg,omitempty"`
 }
 
 // AccessRestrictionHandler is a function that should display a single AccessRestrictionMessage to the user.

diff --git a/pkg/cmd/config/config_test.go b/pkg/cmd/config/config_test.go
@@ -12,7 +12,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/spf13/cobra"
-	"gopkg.in/yaml.v3"
+	"sigs.k8s.io/yaml"
 
 	cmdconfig "github.com/gardener/gardenctl-v2/pkg/cmd/config"
 	"github.com/gardener/gardenctl-v2/pkg/config"

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -15,44 +15,44 @@ import (
 	"strconv"
 
 	"github.com/mitchellh/go-homedir"
-	"gopkg.in/yaml.v3"
 	"k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	"k8s.io/klog/v2"
+	"sigs.k8s.io/yaml"
 
 	"github.com/gardener/gardenctl-v2/pkg/ac"
 )
 
 // Config holds the gardenctl configuration.
 type Config struct {
 	// Filename is the name of the gardenctl configuration file
-	Filename string `yaml:"-" json:"-"`
+	Filename string `json:"-"`
 	// LinkKubeconfig defines if kubeconfig is symlinked with the target
-	LinkKubeconfig *bool `yaml:"linkKubeconfig,omitempty" json:"linkKubeconfig,omitempty"`
+	LinkKubeconfig *bool `json:"linkKubeconfig,omitempty"`
 	// Gardens is a list of known Garden clusters
-	Gardens []Garden `yaml:"gardens" json:"gardens"`
+	Gardens []Garden `json:"gardens"`
 }
 
 // Garden represents one garden cluster.
 type Garden struct {
 	// Name is a unique identifier of this Garden that can be used to target this Garden
-	Name string `yaml:"identity" json:"identity"`
+	Name string `json:"identity"`
 	// Alias is a unique identifier of this Garden that can be used as an alternate name to target this Garden
 	// +optional
-	Alias string `yaml:"name,omitempty" json:"name,omitempty"`
+	Alias string `json:"name,omitempty"`
 	// Kubeconfig holds the path for the kubeconfig of the garden cluster
-	Kubeconfig string `yaml:"kubeconfig" json:"kubeconfig"`
+	Kubeconfig string `json:"kubeconfig"`
 	// Context overrides the current-context of the garden cluster kubeconfig
 	// +optional
-	Context string `yaml:"context,omitempty" json:"context,omitempty"`
+	Context string `json:"context,omitempty"`
 	// Patterns is a list of regex patterns that can be defined to use custom input formats for targeting
 	// Use named capturing groups to match target values.
 	// Supported capturing groups: project, namespace, shoot
 	// +optional
-	Patterns []string `yaml:"patterns,omitempty" json:"patterns,omitempty"`
+	Patterns []string `json:"patterns,omitempty"`
 	// AccessRestrictions is a list of access restriction definitions
 	// +optional
-	AccessRestrictions []ac.AccessRestriction `yaml:"accessRestrictions,omitempty" json:"accessRestrictions,omitempty"`
+	AccessRestrictions []ac.AccessRestriction `json:"accessRestrictions,omitempty"`
 }
 
 // LoadFromFile parses a gardenctl config file and returns a Config struct.
@@ -75,7 +75,12 @@ func LoadFromFile(filename string) (*Config, error) {
 	}
 
 	if stat.Size() > 0 {
-		if err := yaml.NewDecoder(f).Decode(config); err != nil {
+		buf, err := os.ReadFile(filename)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read config file: %w", err)
+		}
+
+		if err = yaml.Unmarshal(buf, config); err != nil {
 			return nil, fmt.Errorf("failed to decode as YAML: %w", err)
 		}
 
@@ -147,14 +152,13 @@ func (config *Config) Save() error {
 		return fmt.Errorf("failed to create directory: %w", err)
 	}
 
-	f, err := os.OpenFile(config.Filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o600)
+	buf, err := yaml.Marshal(config)
 	if err != nil {
-		return fmt.Errorf("failed to create file: %w", err)
+		return fmt.Errorf("failed to encode as YAML: %w", err)
 	}
-	defer f.Close()
 
-	if err := yaml.NewEncoder(f).Encode(config); err != nil {
-		return fmt.Errorf("failed to encode as YAML: %w", err)
+	if err := os.WriteFile(config.Filename, buf, 0o600); err != nil {
+		return fmt.Errorf("failed to write file: %w", err)
 	}
 
 	return nil

diff --git a/pkg/config/config_suite_test.go b/pkg/config/config_suite_test.go
@@ -7,7 +7,6 @@ SPDX-License-Identifier: Apache-2.0
 package config_test
 
 import (
-	"os"
 	"testing"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -18,15 +17,3 @@ func TestConfiguration(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "Config Test Suite")
 }
-
-var gardenHomeDir string
-
-var _ = BeforeSuite(func() {
-	dir, err := os.MkdirTemp("", "garden-*")
-	Expect(err).NotTo(HaveOccurred())
-	gardenHomeDir = dir
-})
-
-var _ = AfterSuite(func() {
-	Expect(os.RemoveAll(gardenHomeDir)).To(Succeed())
-})
diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
@@ -20,6 +20,7 @@ import (
 
 var _ = Describe("Config", func() {
 	var (
+		gardenHomeDir    string
 		clusterIdentity1 = "garden1"
 		clusterIdentity2 = "garden2"
 		clusterAlias1    = "gardenalias1"
@@ -31,6 +32,10 @@ var _ = Describe("Config", func() {
 	)
 
 	BeforeEach(func() {
+		dir, err := os.MkdirTemp("", "garden-*")
+		Expect(err).NotTo(HaveOccurred())
+		gardenHomeDir = dir
+
 		cfg = &config.Config{
 			LinkKubeconfig: pointer.Bool(false),
 			Gardens: []config.Garden{
@@ -53,6 +58,10 @@ var _ = Describe("Config", func() {
 		}
 	})
 
+	AfterEach(func() {
+		Expect(os.RemoveAll(gardenHomeDir)).To(Succeed())
+	})
+
 	patternValue := func(prefix string) string {
 		value := fmt.Sprintf("shoot--%s--%s", project, shoot)
 
@@ -181,4 +190,33 @@ var _ = Describe("Config", func() {
 		}
 		Expect(cfg.Save()).NotTo(HaveOccurred())
 	})
+
+	Describe("#LoadFromFile", func() {
+		It("should succeed when file does not exist", func() {
+			filename := filepath.Join(gardenHomeDir, "gardenctl-v2.yaml")
+			cfg = &config.Config{
+				Filename: filename,
+			}
+
+			cfg, err := config.LoadFromFile(filename)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(cfg.Filename).To(Equal(filename))
+			Expect(cfg.Gardens).To(BeNil())
+		})
+
+		It("should succeed when file is empty", func() {
+			filename := filepath.Join(gardenHomeDir, "gardenctl-v2.yaml")
+			_, err := os.Create(filename)
+			Expect(err).NotTo(HaveOccurred())
+
+			cfg = &config.Config{
+				Filename: filename,
+			}
+
+			cfg, err := config.LoadFromFile(filename)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(cfg.Filename).To(Equal(filename))
+			Expect(cfg.Gardens).To(BeNil())
+		})
+	})
 })
diff --git a/pkg/target/target.go b/pkg/target/target.go
@@ -63,11 +63,11 @@ type Target interface {
 }
 
 type targetImpl struct {
-	Garden           string `yaml:"garden,omitempty" json:"garden,omitempty"`
-	Project          string `yaml:"project,omitempty" json:"project,omitempty"`
-	Seed             string `yaml:"seed,omitempty" json:"seed,omitempty"`
-	Shoot            string `yaml:"shoot,omitempty" json:"shoot,omitempty"`
-	ControlPlaneFlag bool   `yaml:"controlPlane,omitempty" json:"controlPlane,omitempty"`
+	Garden           string `json:"garden,omitempty"`
+	Project          string `json:"project,omitempty"`
+	Seed             string `json:"seed,omitempty"`
+	Shoot            string `json:"shoot,omitempty"`
+	ControlPlaneFlag bool   `json:"controlPlane,omitempty"`
 }
 
 var _ Target = &targetImpl{}

diff --git a/pkg/target/target_provider.go b/pkg/target/target_provider.go
@@ -10,7 +10,7 @@ import (
 	"fmt"
 	"os"
 
-	"gopkg.in/yaml.v3"
+	"sigs.k8s.io/yaml"
 )
 
 // TargetReader can read targets.
@@ -65,8 +65,14 @@ func (p *fsTargetProvider) Read() (Target, error) {
 	}
 
 	target := &targetImpl{}
+
 	if stat.Size() > 0 {
-		if err := yaml.NewDecoder(f).Decode(target); err != nil {
+		buf, err := os.ReadFile(p.targetFile)
+		if err != nil {
+			return nil, fmt.Errorf("failed to read target file: %w", err)
+		}
+
+		if err = yaml.Unmarshal(buf, target); err != nil {
 			return nil, fmt.Errorf("failed to decode as YAML: %w", err)
 		}
 	}
@@ -80,14 +86,13 @@ func (p *fsTargetProvider) Read() (Target, error) {
 
 // Write takes a target and saves it permanently.
 func (p *fsTargetProvider) Write(t Target) error {
-	f, err := os.OpenFile(p.targetFile, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o600)
+	buf, err := yaml.Marshal(t)
 	if err != nil {
-		return fmt.Errorf("failed to create file: %w", err)
+		return fmt.Errorf("failed to encode as YAML: %w", err)
 	}
-	defer f.Close()
 
-	if err := yaml.NewEncoder(f).Encode(t); err != nil {
-		return fmt.Errorf("failed to encode as YAML: %w", err)
+	if err := os.WriteFile(p.targetFile, buf, 0o600); err != nil {
+		return fmt.Errorf("failed to write file: %w", err)
 	}
 
 	return nil