Skip to content

Commit

Permalink
fix: properly determine list objects in schemagen (#3205)
Browse files Browse the repository at this point in the history
CRDs can have types that end in `List`. These are not actually List
resources, so we should not rely on name alone to determine this.
Instead, we should follow the rules set out in the [conventions
guide](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#types-kinds)
to determine is a Kind is meant to be a List kind.
  • Loading branch information
rquitales authored Sep 16, 2024
1 parent 7155e2e commit 07c064d
Show file tree
Hide file tree
Showing 4 changed files with 279 additions and 1 deletion.
4 changes: 3 additions & 1 deletion provider/pkg/gen/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,9 @@ func PulumiSchema(swagger map[string]any, opts ...schemaGeneratorOption) pschema
for _, kind := range version.Kinds() {
tok := fmt.Sprintf(`kubernetes:%s:%s`, kind.apiVersion, kind.kind)
var patchTok string
if !strings.HasSuffix(kind.kind, "List") {

// Generate patch variants for non-list resources.
if !kind.isList {
patchTok = fmt.Sprintf("%sPatch", tok)
}

Expand Down
153 changes: 153 additions & 0 deletions provider/pkg/gen/testdata/identify-list-kinds/list-kind
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
-- definitions --
{
"com.pulumi.k8sversion.test.TestResource": {
"properties": {
"apiVersion": {
"description": "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
"type": "string"
},
"kind": {
"description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
"type": "string"
},
"metadata": {
"description": "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
"type": "object",
"properties": {
"fakeField": {
"type": "string"
}
}
},
"spec": {
"type": "string"
}
},
"x-kubernetes-group-version-kind": [
{
"group": "k8sversion.pulumi.com",
"kind": "TestResource",
"version": "test"
}
],
"x-kubernetes-selectable-fields": []
},
"com.pulumi.k8sversion.test.TestResourceList": {
"description": "TestResourceList is a list of TestResource",
"type": "object",
"properties": {
"apiVersion": {
"description": "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
"type": "string"
},
"items": {
"description": "List of testresources. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md",
"type": "array",
"items": {
"$ref": "#/definitions/com.pulumi.k8sversion.test.TestResource"
}
},
"kind": {
"description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
"type": "string"
},
"metadata": {
"description": "Standard list metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
"type": "object",
"properties": {
"fakeField": {
"type": "string"
}
}
}
},
"x-kubernetes-group-version-kind": [
{
"group": "k8sversion.pulumi.com",
"kind": "TestResourceList",
"version": "test"
}
],
"x-kubernetes-selectable-fields": []
},
"com.pulumi.k8sversion.test.TestResourceNotRealList": {
"description": "TestResourceNotRealList is not a real list object.",
"type": "object",
"properties": {
"apiVersion": {
"description": "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
"type": "string"
},
"kind": {
"description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
"type": "string"
},
"metadata": {
"description": "Standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
"type": "object",
"properties": {
"fakeField": {
"type": "string"
}
}
},
"spec": {
"type": "string"
}
},
"x-kubernetes-group-version-kind": [
{
"group": "k8sversion.pulumi.com",
"kind": "TestResourceNotRealList",
"version": "test"
}
],
"x-kubernetes-selectable-fields": []
},
"com.pulumi.k8sversion.test.TestResourceNotRealListList": {
"description": "TestResourceNotRealListList is a list of TestResourceNotRealList",
"type": "object",
"properties": {
"apiVersion": {
"description": "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
"type": "string"
},
"items": {
"description": "List of testresourcelists. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md",
"type": "array",
"items": {
"$ref": "#/definitions/com.pulumi.k8sversion.test.TestResourceNotRealList"
}
},
"kind": {
"description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
"type": "string"
},
"metadata": {
"description": "Standard list metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
"type": "object",
"properties": {
"fakeField": {
"type": "string"
}
}
}
},
"x-kubernetes-group-version-kind": [
{
"group": "k8sversion.pulumi.com",
"kind": "TestResourceNotRealListList",
"version": "test"
}
],
"x-kubernetes-selectable-fields": []
}
}

-- kinds --
- k8sversion.test.TestResource
- k8sversion.test.TestResourceNotRealList

-- listKinds --
- k8sversion.test.TestResourceList
- k8sversion.test.TestResourceNotRealListList
21 changes: 21 additions & 0 deletions provider/pkg/gen/typegen.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ type KindConfig struct {
defaultAPIVersion string

isNested bool
isList bool // Indicates if this kind is a list.

schemaPkgName string
}
Expand Down Expand Up @@ -486,6 +487,7 @@ func createGroups(definitionsJSON map[string]any, allowHyphens bool) []GroupConf
}
def.canonicalGroup = group
}

return def
}).
WhereT(func(d definition) bool { return d.canonicalGroup != "" }).
Expand Down Expand Up @@ -593,13 +595,23 @@ func createGroups(definitionsJSON map[string]any, allowHyphens bool) []GroupConf

defaultAPIVersion := d.defaultAPIVersion()
isTopLevel := d.isTopLevel()
isList := false

ps := linq.From(d.data["properties"]).
OrderByT(func(kv linq.KeyValue) string { return kv.Key.(string) }).
SelectT(func(kv linq.KeyValue) Property {
propName := kv.Key.(string)
prop := d.data["properties"].(map[string]any)[propName].(map[string]any)

// Determine if kind is a list resource if it contains an `items` property that is an array and Kind name ends in `List`.
// Ref: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#types-kinds
propType, ok := prop["type"].(string)
if ok {
if propName == "items" && propType == "array" && strings.HasSuffix(d.gvk.Kind, "List") {
isList = true
}
}

schemaType := makeSchemaType(prop, canonicalGroups)

// `-` is invalid in variable names, so replace with `_`
Expand Down Expand Up @@ -693,6 +705,14 @@ func createGroups(definitionsJSON map[string]any, allowHyphens bool) []GroupConf

return fmt.Sprintf("%s/%s", gStripped, v)
}

// These resources are hard-coded as lists as they do not adhere to the normal conventions.
if d.gvk.Group == "meta" &&
d.gvk.Version == "v1" &&
(d.gvk.Kind == "APIResourceList" || d.gvk.Kind == "APIGroupList") {
isList = true
}

return linq.From([]KindConfig{
{
kind: d.gvk.Kind,
Expand All @@ -708,6 +728,7 @@ func createGroups(definitionsJSON map[string]any, allowHyphens bool) []GroupConf
defaultAPIVersion: defaultAPIVersion,
isNested: !isTopLevel,
schemaPkgName: schemaPkgName(apiVersion),
isList: isList,
},
})
}).
Expand Down
102 changes: 102 additions & 0 deletions provider/pkg/gen/typegen_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Copyright 2016-2024, Pulumi Corporation.
//
// 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.

// nolint: lll
package gen

import (
"encoding/json"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.org/x/tools/txtar"
"gopkg.in/yaml.v2"
)

// TestCreateGroups_IdentifyListKinds loads txtar files under testdata/identify-list-kinds and uses them to
// craft an unstructured map[string]any definitions file for createGroups.
// The goal of this test is to ensure we can accurately distinuguish between singletons and lists of kinds.
//
// The test files should contain the following files:
// - definitions: a JSON file containing the definitions
// - kinds: a YAML file containing a list of kinds that are singletons
// - listKinds: a YAML file containing a list of kinds that are lists/collections of singleton kinds
func TestCreateGroups_IdentifyListKinds(t *testing.T) {
dir := filepath.Join("testdata/identify-list-kinds")
tests, err := os.ReadDir(dir)
require.NoError(t, err)

for _, tt := range tests {
t.Run(tt.Name(), func(t *testing.T) {
archive, err := txtar.ParseFile(filepath.Join(dir, tt.Name()))
require.NoError(t, err)

var definitions map[string]any
var kinds, listKinds map[string]struct{}

for _, f := range archive.Files {
var parsed []string
switch f.Name {
case "definitions":
err := json.Unmarshal(f.Data, &definitions)
require.NoError(t, err, f.Name)
case "kinds":
err = yaml.Unmarshal(f.Data, &parsed)
require.NoError(t, err, f.Name)
kinds = sliceToSet(parsed)
case "listKinds":
err = yaml.Unmarshal(f.Data, &parsed)
require.NoError(t, err, f.Name)
listKinds = sliceToSet(parsed)
default:
t.Fatal("unrecognized filename", f.Name)
}
}

configGroups := createGroups(definitions, true)

// Loop through all parsed kinds and ensure they are accounted for.
for _, g := range configGroups {
for _, v := range g.versions {
for _, kind := range v.kinds {
gvk := gvkToString(kind.gvk.Group, kind.gvk.Version, kind.gvk.Kind)
if kind.isList {
delete(listKinds, gvk)
} else {
delete(kinds, gvk)
}
}
}
}

assert.Equal(t, 0, len(kinds), "kinds not found while parsing: %v", kinds)
assert.Equal(t, 0, len(listKinds), "listKinds not found while parsing: %v", listKinds)
})
}
}

func gvkToString(group, version, kind string) string {
return group + "." + version + "." + kind
}

func sliceToSet(slice []string) map[string]struct{} {
set := make(map[string]struct{})
for _, item := range slice {
set[item] = struct{}{}
}
return set
}

0 comments on commit 07c064d

Please sign in to comment.