Merge pull request #58535 from bowei/check-key

Automatic merge from submit-queue (batch tested with PRs 53895, 58013, 58466, 58531, 58535). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

GCE: check key is valid when calling the API

GCE: check key is valid when calling the API

```release-note
NONE
```

pkg/cloudprovider/providers/gce/cloud/gen/main.go

@@ -395,6 +395,9 @@ func (m *{{.MockWrapType}}) Get(ctx context.Context, key *meta.Key) (*{{.FQObjec
 			return obj, err
 		}
 	}
+	if ! key.Valid() {
+		return nil, fmt.Errorf("invalid GCE key (%+v)", key)
+	}
 
 	m.Lock.Lock()
 	defer m.Lock.Unlock()
@@ -510,6 +513,9 @@ func (m *{{.MockWrapType}}) Insert(ctx context.Context, key *meta.Key, obj *{{.F
 			return err
 		}
 	}
+	if ! key.Valid() {
+		return fmt.Errorf("invalid GCE key (%+v)", key)
+	}
 
 	m.Lock.Lock()
 	defer m.Lock.Unlock()
@@ -547,6 +553,9 @@ func (m *{{.MockWrapType}}) Delete(ctx context.Context, key *meta.Key) error {
 			return err
 		}
 	}
+	if ! key.Valid() {
+		return fmt.Errorf("invalid GCE key (%+v)", key)
+	}
 
 	m.Lock.Lock()
 	defer m.Lock.Unlock()
@@ -648,6 +657,9 @@ type {{.GCEWrapType}} struct {
 {{- if .GenerateGet}}
 // Get the {{.Object}} named by key.
 func (g *{{.GCEWrapType}}) Get(ctx context.Context, key *meta.Key) (*{{.FQObjectType}}, error) {
+	if ! key.Valid() {
+		return nil, fmt.Errorf("invalid GCE key (%+v)", key)
+	}
 	projectID := g.s.ProjectRouter.ProjectID(ctx, "{{.Version}}", "{{.Service}}")
 	rk := &RateLimitKey{
 		ProjectID: projectID,
@@ -720,6 +732,9 @@ rk := &RateLimitKey{
 {{- if .GenerateInsert}}
 // Insert {{.Object}} with key of value obj.
 func (g *{{.GCEWrapType}}) Insert(ctx context.Context, key *meta.Key, obj *{{.FQObjectType}}) error {
+	if ! key.Valid() {
+		return fmt.Errorf("invalid GCE key (%+v)", key)
+	}
 	projectID := g.s.ProjectRouter.ProjectID(ctx, "{{.Version}}", "{{.Service}}")
 	rk := &RateLimitKey{
 		ProjectID: projectID,
@@ -753,6 +768,9 @@ func (g *{{.GCEWrapType}}) Insert(ctx context.Context, key *meta.Key, obj *{{.FQ
 {{- if .GenerateDelete}}
 // Delete the {{.Object}} referenced by key.
 func (g *{{.GCEWrapType}}) Delete(ctx context.Context, key *meta.Key) error {
+	if ! key.Valid() {
+		return fmt.Errorf("invalid GCE key (%+v)", key)
+	}
 	projectID := g.s.ProjectRouter.ProjectID(ctx, "{{.Version}}", "{{.Service}}")
 	rk := &RateLimitKey{
 		ProjectID: projectID,
@@ -820,6 +838,15 @@ func (g *{{.GCEWrapType}}) AggregatedList(ctx context.Context, fl *filter.F) (ma
 {{- range .}}
 // {{.Name}} is a method on {{.GCEWrapType}}.
 func (g *{{.GCEWrapType}}) {{.FcnArgs}} {
+	if ! key.Valid() {
+{{- if .IsOperation}}
+		return fmt.Errorf("invalid GCE key (%+v)", key)
+{{- else if .IsGet}}
+		return nil, fmt.Errorf("invalid GCE key (%+v)", key)
+{{- else if .IsPaged}}
+		return nil, fmt.Errorf("invalid GCE key (%+v)", key)
+{{- end}}
+	}
 	projectID := g.s.ProjectRouter.ProjectID(ctx, "{{.Version}}", "{{.Service}}")
 	rk := &RateLimitKey{
 		ProjectID: projectID,

pkg/cloudprovider/providers/gce/cloud/meta/key.go

@@ -18,6 +18,7 @@ package meta
 
 import (
 	"fmt"
+	"regexp"
 )
 
 // Key for a GCP resource.
@@ -39,6 +40,11 @@ const (
 	Global = "global"
 )
 
+var (
+	// locationRegexp is the format of regions/zone names in GCE.
+	locationRegexp = regexp.MustCompile("^[a-z](?:[-a-z0-9]+)?$")
+)
+
 // ZonalKey returns the key for a zonal resource.
 func ZonalKey(name, zone string) *Key {
 	return &Key{name, zone, ""}
@@ -79,10 +85,16 @@ func (k Key) String() string {
 }
 
 // Valid is true if the key is valid.
-func (k *Key) Valid(typeName string) bool {
+func (k *Key) Valid() bool {
 	if k.Zone != "" && k.Region != "" {
 		return false
 	}
+	switch {
+	case k.Region != "":
+		return locationRegexp.Match([]byte(k.Region))
+	case k.Zone != "":
+		return locationRegexp.Match([]byte(k.Zone))
+	}
 	return true
 }
 

pkg/cloudprovider/providers/gce/cloud/meta/key_test.go

@@ -58,18 +58,19 @@ func TestKeyValid(t *testing.T) {
 	zone := "us-central1-b"
 
 	for _, tc := range []struct {
-		key      *Key
-		typeName string
-		want     bool
+		key  *Key
+		want bool
 	}{
-		// Note: these test cases need to be synchronized with the
-		// actual settings for each type.
-		{GlobalKey("abc"), "UrlMap", true},
-		{&Key{"abc", zone, region}, "UrlMap", false},
+		{GlobalKey("abc"), true},
+		{RegionalKey("abc", region), true},
+		{ZonalKey("abc", zone), true},
+		{RegionalKey("abc", "/invalid/"), false},
+		{ZonalKey("abc", "/invalid/"), false},
+		{&Key{"abc", zone, region}, false},
 	} {
-		valid := tc.key.Valid(tc.typeName)
-		if valid != tc.want {
-			t.Errorf("key %+v, type %v; key.Valid() = %v, want %v", tc.key, tc.typeName, valid, tc.want)
+		got := tc.key.Valid()
+		if got != tc.want {
+			t.Errorf("key %+v; key.Valid() = %v, want %v", tc.key, got, tc.want)
 		}
 	}
 }