Skip to content

Commit 1c247e7

Browse files
josvazgjosvazg
authored
CLOUDP-366839: Fix CRD print warnings (#3044)
* Place printer columns at the version level * Place printer columns at the spec level * Add explanatory comment Signed-off-by: jose.vazquez <[email protected]> --------- Signed-off-by: jose.vazquez <[email protected]>
1 parent 918c505 commit 1c247e7

File tree

3 files changed

+53
-73
lines changed

3 files changed

+53
-73
lines changed

‎internal/generated/crds/crds.yaml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ spec:
2929
- jsonPath: .status.conditions[?(@.type=="Ready")].status
3030
name: Ready
3131
type: string
32+
- jsonPath: .status.conditions[?(@.type=="Ready")].reason
33+
name: Reason
34+
type: string
3235
- jsonPath: .status.conditions[?(@.type=="State")].reason
3336
name: State
3437
type: string
@@ -246,6 +249,9 @@ spec:
246249
- jsonPath: .status.conditions[?(@.type=="Ready")].status
247250
name: Ready
248251
type: string
252+
- jsonPath: .status.conditions[?(@.type=="Ready")].reason
253+
name: Reason
254+
type: string
249255
- jsonPath: .status.conditions[?(@.type=="State")].reason
250256
name: State
251257
type: string
@@ -1573,6 +1579,9 @@ spec:
15731579
- jsonPath: .status.conditions[?(@.type=="Ready")].status
15741580
name: Ready
15751581
type: string
1582+
- jsonPath: .status.conditions[?(@.type=="Ready")].reason
1583+
name: Reason
1584+
type: string
15761585
- jsonPath: .status.conditions[?(@.type=="State")].reason
15771586
name: State
15781587
type: string
@@ -1905,6 +1914,9 @@ spec:
19051914
- jsonPath: .status.conditions[?(@.type=="Ready")].status
19061915
name: Ready
19071916
type: string
1917+
- jsonPath: .status.conditions[?(@.type=="Ready")].reason
1918+
name: Reason
1919+
type: string
19081920
- jsonPath: .status.conditions[?(@.type=="State")].reason
19091921
name: State
19101922
type: string

‎tools/openapi2crd/pkg/plugins/print_conditions.go‎

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package plugins
22

33
import (
4-
"fmt"
5-
64
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
75
)
86

@@ -13,16 +11,28 @@ func (p *PrintConditions) Name() string {
1311
}
1412

1513
func (p *PrintConditions) Process(req *MappingProcessorRequest) error {
16-
index := findVersionIndex(req.CRD.Spec.Versions, req.CRD.APIVersion)
17-
if index == -1 {
18-
return fmt.Errorf("apiVersion %q not listed in spec", req.CRD.APIVersion)
19-
}
20-
req.CRD.Spec.Versions[index].AdditionalPrinterColumns = []apiextensions.CustomResourceColumnDefinition{
14+
// Given that new SDK versions are supported by different spec subfields, it
15+
// is unlikely there will be more than one supported Kubernetes version.
16+
//
17+
// The CRD validation fails if all versions use the same print columns, in
18+
// such case they have to be set at the top level to avoid errors.
19+
// Note that the produced YAML still places the print columns at the version
20+
// level anyways.
21+
//
22+
// Check out the upstream code for more details:
23+
// https://github.com/kubernetes/apiextensions-apiserver/blob/a780e0393511161d7ef1e6466035181a4f84f347/pkg/apis/apiextensions/validation/validation.go#L438C5-L438C34
24+
// https://github.com/kubernetes/apiextensions-apiserver/blob/a780e0393511161d7ef1e6466035181a4f84f347/pkg/apis/apiextensions/validation/validation.go#L762
25+
req.CRD.Spec.AdditionalPrinterColumns = []apiextensions.CustomResourceColumnDefinition{
2126
{
2227
JSONPath: `.status.conditions[?(@.type=="Ready")].status`,
2328
Name: "Ready",
2429
Type: "string",
2530
},
31+
{
32+
JSONPath: `.status.conditions[?(@.type=="Ready")].reason`,
33+
Name: "Reason",
34+
Type: "string",
35+
},
2636
{
2737
JSONPath: `.status.conditions[?(@.type=="State")].reason`,
2838
Name: "State",
@@ -31,15 +41,3 @@ func (p *PrintConditions) Process(req *MappingProcessorRequest) error {
3141
}
3242
return nil
3343
}
34-
35-
func findVersionIndex(versions []apiextensions.CustomResourceDefinitionVersion, version string) int {
36-
if len(versions) == 1 && version == "" {
37-
return 0
38-
}
39-
for i, v := range versions {
40-
if v.Name == version {
41-
return i
42-
}
43-
}
44-
return -1
45-
}

‎tools/openapi2crd/pkg/plugins/print_conditions_test.go‎

Lines changed: 24 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -25,87 +25,57 @@ func TestPrintConditions_Process(t *testing.T) {
2525
validate func(t *testing.T, crd *apiextensions.CustomResourceDefinition)
2626
}{
2727
{
28-
title: "version updated",
28+
title: "columns added to spec",
2929
apiVersion: "v1",
3030
versions: []apiextensions.CustomResourceDefinitionVersion{
3131
{Name: "v1beta1", Served: true, Storage: false},
3232
{Name: "v1", Served: true, Storage: true},
3333
},
3434
validate: func(t *testing.T, crd *apiextensions.CustomResourceDefinition) {
35-
// Find the v1 version
36-
var targetVersion apiextensions.CustomResourceDefinitionVersion
37-
for _, v := range crd.Spec.Versions {
38-
if v.Name == "v1" {
39-
targetVersion = v
40-
break
41-
}
42-
}
35+
// Verify columns were added at the spec level
36+
require.Len(t, crd.Spec.AdditionalPrinterColumns, 3)
4337

44-
// Verify columns were added
45-
require.NotEmpty(t, targetVersion.Name, "Should have found the target version")
46-
require.Len(t, targetVersion.AdditionalPrinterColumns, 2)
38+
assert.Equal(t, "Ready", crd.Spec.AdditionalPrinterColumns[0].Name)
39+
assert.Equal(t, `.status.conditions[?(@.type=="Ready")].status`, crd.Spec.AdditionalPrinterColumns[0].JSONPath)
4740

48-
assert.Equal(t, "Ready", targetVersion.AdditionalPrinterColumns[0].Name)
49-
assert.Equal(t, `.status.conditions[?(@.type=="Ready")].status`, targetVersion.AdditionalPrinterColumns[0].JSONPath)
41+
assert.Equal(t, "Reason", crd.Spec.AdditionalPrinterColumns[1].Name)
42+
assert.Equal(t, `.status.conditions[?(@.type=="Ready")].reason`, crd.Spec.AdditionalPrinterColumns[1].JSONPath)
5043

51-
assert.Equal(t, "State", targetVersion.AdditionalPrinterColumns[1].Name)
52-
assert.Equal(t, `.status.conditions[?(@.type=="State")].reason`, targetVersion.AdditionalPrinterColumns[1].JSONPath)
44+
assert.Equal(t, "State", crd.Spec.AdditionalPrinterColumns[2].Name)
45+
assert.Equal(t, `.status.conditions[?(@.type=="State")].reason`, crd.Spec.AdditionalPrinterColumns[2].JSONPath)
5346
},
5447
},
5548
{
56-
title: "missing version",
57-
apiVersion: "v2",
58-
versions: []apiextensions.CustomResourceDefinitionVersion{
59-
{Name: "v1", Served: true, Storage: true},
60-
},
61-
wantErr: "apiVersion \"v2\" not listed in spec",
62-
validate: func(t *testing.T, crd *apiextensions.CustomResourceDefinition) {
63-
// Ensure no weird side effects happened to existing versions
64-
assert.Empty(t, crd.Spec.Versions[0].AdditionalPrinterColumns)
65-
},
66-
},
67-
{
68-
title: "replace columns",
49+
title: "replace existing columns",
6950
apiVersion: "v1",
7051
versions: []apiextensions.CustomResourceDefinitionVersion{
71-
{
72-
Name: "v1",
73-
AdditionalPrinterColumns: []apiextensions.CustomResourceColumnDefinition{
74-
{Name: "OldColumn", JSONPath: ".old"},
75-
},
76-
},
52+
{Name: "v1", Served: true, Storage: true},
7753
},
7854
validate: func(t *testing.T, crd *apiextensions.CustomResourceDefinition) {
79-
targetVersion := crd.Spec.Versions[0]
80-
require.Len(t, targetVersion.AdditionalPrinterColumns, 2)
81-
assert.Equal(t, "Ready", targetVersion.AdditionalPrinterColumns[0].Name)
55+
require.Len(t, crd.Spec.AdditionalPrinterColumns, 3)
56+
assert.Equal(t, "Ready", crd.Spec.AdditionalPrinterColumns[0].Name)
57+
assert.Equal(t, "Reason", crd.Spec.AdditionalPrinterColumns[1].Name)
58+
assert.Equal(t, "State", crd.Spec.AdditionalPrinterColumns[2].Name)
8259
},
8360
},
8461
{
85-
title: "unique version updated",
62+
title: "columns added with single version",
8663
apiVersion: "",
8764
versions: []apiextensions.CustomResourceDefinitionVersion{
8865
{Name: "v1", Served: true, Storage: true},
8966
},
9067
validate: func(t *testing.T, crd *apiextensions.CustomResourceDefinition) {
91-
// Find the v1 version
92-
var targetVersion apiextensions.CustomResourceDefinitionVersion
93-
for _, v := range crd.Spec.Versions {
94-
if v.Name == "v1" {
95-
targetVersion = v
96-
break
97-
}
98-
}
68+
// Verify columns were added at the spec level
69+
require.Len(t, crd.Spec.AdditionalPrinterColumns, 3)
9970

100-
// Verify columns were added
101-
require.NotEmpty(t, targetVersion.Name, "Should have found the target version")
102-
require.Len(t, targetVersion.AdditionalPrinterColumns, 2)
71+
assert.Equal(t, "Ready", crd.Spec.AdditionalPrinterColumns[0].Name)
72+
assert.Equal(t, `.status.conditions[?(@.type=="Ready")].status`, crd.Spec.AdditionalPrinterColumns[0].JSONPath)
10373

104-
assert.Equal(t, "Ready", targetVersion.AdditionalPrinterColumns[0].Name)
105-
assert.Equal(t, `.status.conditions[?(@.type=="Ready")].status`, targetVersion.AdditionalPrinterColumns[0].JSONPath)
74+
assert.Equal(t, "Reason", crd.Spec.AdditionalPrinterColumns[1].Name)
75+
assert.Equal(t, `.status.conditions[?(@.type=="Ready")].reason`, crd.Spec.AdditionalPrinterColumns[1].JSONPath)
10676

107-
assert.Equal(t, "State", targetVersion.AdditionalPrinterColumns[1].Name)
108-
assert.Equal(t, `.status.conditions[?(@.type=="State")].reason`, targetVersion.AdditionalPrinterColumns[1].JSONPath)
77+
assert.Equal(t, "State", crd.Spec.AdditionalPrinterColumns[2].Name)
78+
assert.Equal(t, `.status.conditions[?(@.type=="State")].reason`, crd.Spec.AdditionalPrinterColumns[2].JSONPath)
10979
},
11080
},
11181
}

0 commit comments

Comments
 (0)