Skip to content

Commit

Permalink
Improve error handling for List resources (#2493)
Browse files Browse the repository at this point in the history
List resources are included in the SDK for compatibility with various YAML manifests and Helm charts. These resources are not currently able to be created directly, but the failure condition was not handled explicitly. Instead, users would get an error when the provider failed to determine if the resource was namespaced. This is fixed with the following changes:

1. Create a lookup table for known List resources
2. Return an error during Check for List resources with a descriptive error message

This change also adds a few missing kinds to the Namespaced lookup table that have been added since the table was created initially.
  • Loading branch information
lblackstone committed Jul 17, 2023
1 parent c628a79 commit a160f74
Show file tree
Hide file tree
Showing 8 changed files with 254 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Other changes:
- Fix a bug with the diff calculation so that resource drift is detected accurately after a refresh.
- Update go module version to v4 (https://github.com/pulumi/pulumi-kubernetes/pull/2466)
- Upgrade to latest helm dependency (https://github.com/pulumi/pulumi-kubernetes/pull/2474)
- Improve error handling for List resources (https://github.com/pulumi/pulumi-kubernetes/pull/2493)

## 3.30.2 (July 11, 2023)

Expand Down
7 changes: 5 additions & 2 deletions provider/cmd/pulumi-gen-kubernetes/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ func mustRenderGoTemplate(path string, resources any) []byte {
}

func genK8sResourceTypes(pkg *schema.Package) {
groupVersions, kinds, patchKinds := codegen.NewStringSet(), codegen.NewStringSet(), codegen.NewStringSet()
groupVersions, kinds, patchKinds, listKinds := codegen.NewStringSet(), codegen.NewStringSet(), codegen.NewStringSet(), codegen.NewStringSet()
for _, resource := range pkg.Resources {
if resourcesToFilterFromTemplate.Has(resource.Token) {
continue
Expand All @@ -458,12 +458,15 @@ func genK8sResourceTypes(pkg *schema.Package) {
patchKinds.Add(resource.Token)
continue
}
if strings.HasSuffix(kind, "List") {
listKinds.Add(resource.Token)
}

groupVersions.Add(groupVersion)
kinds.Add(kind)
}

gvk := gen.GVK{Kinds: kinds.SortedValues(), PatchKinds: patchKinds.SortedValues()}
gvk := gen.GVK{Kinds: kinds.SortedValues(), PatchKinds: patchKinds.SortedValues(), ListKinds: listKinds.SortedValues()}
gvStrings := groupVersions.SortedValues()
for _, gvString := range gvStrings {
gvk.GroupVersions = append(gvk.GroupVersions, gen.GroupVersion(gvString))
Expand Down
17 changes: 16 additions & 1 deletion provider/pkg/gen/kinds/kinds.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,16 @@ const (
// known return value will be false, and the namespaced value is unknown. In this case, this information can be
// queried separately from the k8s API server.
func (k Kind) Namespaced() (known bool, namespaced bool) {
switch k {
baseKind := Kind(strings.TrimSuffix(string(k), "List"))

switch baseKind {
// Note: Use `kubectl api-resources --namespaced=true -o name` to retrieve a list of namespace-scoped resources.
// Ignoring the same imperative endpoints as in `pkg/gen/typegen.go:isTopLevel`
case Binding,
ConfigMap,
ControllerRevision,
CronJob,
CSIStorageCapacity,
DaemonSet,
Deployment,
Endpoints,
Expand All @@ -69,20 +73,23 @@ func (k Kind) Namespaced() (known bool, namespaced bool) {
StatefulSet:
return true, true
// Note: Use `kubectl api-resources --namespaced=false -o name` to retrieve a list of cluster-scoped resources.
// Ignoring the same imperative endpoints as in `pkg/gen/typegen.go:isTopLevel`
case APIService,
CertificateSigningRequest,
ClusterRole,
ClusterRoleBinding,
CSIDriver,
CSINode,
CustomResourceDefinition,
FlowSchema,
IngressClass,
MutatingWebhookConfiguration,
Namespace,
Node,
PersistentVolume,
PodSecurityPolicy,
PriorityClass,
PriorityLevelConfiguration,
RuntimeClass,
StorageClass,
ValidatingWebhookConfiguration,
Expand Down Expand Up @@ -129,3 +136,11 @@ var PatchQualifiedTypes = codegen.NewStringSet(
"{{.}}",
{{- end}}
)

// ListQualifiedTypes is the set of "List" resource QualifiedType URN tokens. Checking against this known set rather
// than using the List suffix avoids unintended clashes with CustomResources that also contain a List suffix.
var ListQualifiedTypes = codegen.NewStringSet(
{{- range .ListKinds}}
"{{.}}",
{{- end}}
)
1 change: 1 addition & 0 deletions provider/pkg/gen/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ type GVK struct {
GroupVersions []GroupVersion
Kinds []string
PatchKinds []string
ListKinds []string
}

// GroupVersion is the GroupVersion for a k8s resource.
Expand Down
132 changes: 131 additions & 1 deletion provider/pkg/kinds/kinds.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,16 @@ const (
// known return value will be false, and the namespaced value is unknown. In this case, this information can be
// queried separately from the k8s API server.
func (k Kind) Namespaced() (known bool, namespaced bool) {
switch k {
baseKind := Kind(strings.TrimSuffix(string(k), "List"))

switch baseKind {
// Note: Use `kubectl api-resources --namespaced=true -o name` to retrieve a list of namespace-scoped resources.
// Ignoring the same imperative endpoints as in `pkg/gen/typegen.go:isTopLevel`
case Binding,
ConfigMap,
ControllerRevision,
CronJob,
CSIStorageCapacity,
DaemonSet,
Deployment,
Endpoints,
Expand All @@ -188,20 +192,23 @@ func (k Kind) Namespaced() (known bool, namespaced bool) {
StatefulSet:
return true, true
// Note: Use `kubectl api-resources --namespaced=false -o name` to retrieve a list of cluster-scoped resources.
// Ignoring the same imperative endpoints as in `pkg/gen/typegen.go:isTopLevel`
case APIService,
CertificateSigningRequest,
ClusterRole,
ClusterRoleBinding,
CSIDriver,
CSINode,
CustomResourceDefinition,
FlowSchema,
IngressClass,
MutatingWebhookConfiguration,
Namespace,
Node,
PersistentVolume,
PodSecurityPolicy,
PriorityClass,
PriorityLevelConfiguration,
RuntimeClass,
StorageClass,
ValidatingWebhookConfiguration,
Expand Down Expand Up @@ -467,3 +474,126 @@ var PatchQualifiedTypes = codegen.NewStringSet(
"kubernetes:storage.k8s.io/v1beta1:StorageClassPatch",
"kubernetes:storage.k8s.io/v1beta1:VolumeAttachmentPatch",
)

// ListQualifiedTypes is the set of "List" resource QualifiedType URN tokens. Checking against this known set rather
// than using the List suffix avoids unintended clashes with CustomResources that also contain a List suffix.
var ListQualifiedTypes = codegen.NewStringSet(
"kubernetes:admissionregistration.k8s.io/v1:MutatingWebhookConfigurationList",
"kubernetes:admissionregistration.k8s.io/v1:ValidatingWebhookConfigurationList",
"kubernetes:admissionregistration.k8s.io/v1alpha1:ValidatingAdmissionPolicyBindingList",
"kubernetes:admissionregistration.k8s.io/v1alpha1:ValidatingAdmissionPolicyList",
"kubernetes:admissionregistration.k8s.io/v1beta1:MutatingWebhookConfigurationList",
"kubernetes:admissionregistration.k8s.io/v1beta1:ValidatingWebhookConfigurationList",
"kubernetes:apiextensions.k8s.io/v1:CustomResourceDefinitionList",
"kubernetes:apiextensions.k8s.io/v1beta1:CustomResourceDefinitionList",
"kubernetes:apiregistration.k8s.io/v1:APIServiceList",
"kubernetes:apiregistration.k8s.io/v1beta1:APIServiceList",
"kubernetes:apps/v1:ControllerRevisionList",
"kubernetes:apps/v1:DaemonSetList",
"kubernetes:apps/v1:DeploymentList",
"kubernetes:apps/v1:ReplicaSetList",
"kubernetes:apps/v1:StatefulSetList",
"kubernetes:apps/v1beta1:ControllerRevisionList",
"kubernetes:apps/v1beta1:DeploymentList",
"kubernetes:apps/v1beta1:StatefulSetList",
"kubernetes:apps/v1beta2:ControllerRevisionList",
"kubernetes:apps/v1beta2:DaemonSetList",
"kubernetes:apps/v1beta2:DeploymentList",
"kubernetes:apps/v1beta2:ReplicaSetList",
"kubernetes:apps/v1beta2:StatefulSetList",
"kubernetes:auditregistration.k8s.io/v1alpha1:AuditSinkList",
"kubernetes:autoscaling/v1:HorizontalPodAutoscalerList",
"kubernetes:autoscaling/v2:HorizontalPodAutoscalerList",
"kubernetes:autoscaling/v2beta1:HorizontalPodAutoscalerList",
"kubernetes:autoscaling/v2beta2:HorizontalPodAutoscalerList",
"kubernetes:batch/v1:CronJobList",
"kubernetes:batch/v1:JobList",
"kubernetes:batch/v1beta1:CronJobList",
"kubernetes:batch/v2alpha1:CronJobList",
"kubernetes:certificates.k8s.io/v1:CertificateSigningRequestList",
"kubernetes:certificates.k8s.io/v1alpha1:ClusterTrustBundleList",
"kubernetes:certificates.k8s.io/v1beta1:CertificateSigningRequestList",
"kubernetes:coordination.k8s.io/v1:LeaseList",
"kubernetes:coordination.k8s.io/v1beta1:LeaseList",
"kubernetes:core/v1:ConfigMapList",
"kubernetes:core/v1:EndpointsList",
"kubernetes:core/v1:EventList",
"kubernetes:core/v1:LimitRangeList",
"kubernetes:core/v1:NamespaceList",
"kubernetes:core/v1:NodeList",
"kubernetes:core/v1:PersistentVolumeClaimList",
"kubernetes:core/v1:PersistentVolumeList",
"kubernetes:core/v1:PodList",
"kubernetes:core/v1:PodTemplateList",
"kubernetes:core/v1:ReplicationControllerList",
"kubernetes:core/v1:ResourceQuotaList",
"kubernetes:core/v1:SecretList",
"kubernetes:core/v1:ServiceAccountList",
"kubernetes:core/v1:ServiceList",
"kubernetes:discovery.k8s.io/v1:EndpointSliceList",
"kubernetes:discovery.k8s.io/v1beta1:EndpointSliceList",
"kubernetes:events.k8s.io/v1:EventList",
"kubernetes:events.k8s.io/v1beta1:EventList",
"kubernetes:extensions/v1beta1:DaemonSetList",
"kubernetes:extensions/v1beta1:DeploymentList",
"kubernetes:extensions/v1beta1:IngressList",
"kubernetes:extensions/v1beta1:NetworkPolicyList",
"kubernetes:extensions/v1beta1:PodSecurityPolicyList",
"kubernetes:extensions/v1beta1:ReplicaSetList",
"kubernetes:flowcontrol.apiserver.k8s.io/v1alpha1:FlowSchemaList",
"kubernetes:flowcontrol.apiserver.k8s.io/v1alpha1:PriorityLevelConfigurationList",
"kubernetes:flowcontrol.apiserver.k8s.io/v1beta1:FlowSchemaList",
"kubernetes:flowcontrol.apiserver.k8s.io/v1beta1:PriorityLevelConfigurationList",
"kubernetes:flowcontrol.apiserver.k8s.io/v1beta2:FlowSchemaList",
"kubernetes:flowcontrol.apiserver.k8s.io/v1beta2:PriorityLevelConfigurationList",
"kubernetes:flowcontrol.apiserver.k8s.io/v1beta3:FlowSchemaList",
"kubernetes:flowcontrol.apiserver.k8s.io/v1beta3:PriorityLevelConfigurationList",
"kubernetes:networking.k8s.io/v1:IngressClassList",
"kubernetes:networking.k8s.io/v1:IngressList",
"kubernetes:networking.k8s.io/v1:NetworkPolicyList",
"kubernetes:networking.k8s.io/v1alpha1:ClusterCIDRList",
"kubernetes:networking.k8s.io/v1alpha1:IPAddressList",
"kubernetes:networking.k8s.io/v1beta1:IngressClassList",
"kubernetes:networking.k8s.io/v1beta1:IngressList",
"kubernetes:node.k8s.io/v1:RuntimeClassList",
"kubernetes:node.k8s.io/v1alpha1:RuntimeClassList",
"kubernetes:node.k8s.io/v1beta1:RuntimeClassList",
"kubernetes:policy/v1:PodDisruptionBudgetList",
"kubernetes:policy/v1beta1:PodDisruptionBudgetList",
"kubernetes:policy/v1beta1:PodSecurityPolicyList",
"kubernetes:rbac.authorization.k8s.io/v1:ClusterRoleBindingList",
"kubernetes:rbac.authorization.k8s.io/v1:ClusterRoleList",
"kubernetes:rbac.authorization.k8s.io/v1:RoleBindingList",
"kubernetes:rbac.authorization.k8s.io/v1:RoleList",
"kubernetes:rbac.authorization.k8s.io/v1alpha1:ClusterRoleBindingList",
"kubernetes:rbac.authorization.k8s.io/v1alpha1:ClusterRoleList",
"kubernetes:rbac.authorization.k8s.io/v1alpha1:RoleBindingList",
"kubernetes:rbac.authorization.k8s.io/v1alpha1:RoleList",
"kubernetes:rbac.authorization.k8s.io/v1beta1:ClusterRoleBindingList",
"kubernetes:rbac.authorization.k8s.io/v1beta1:ClusterRoleList",
"kubernetes:rbac.authorization.k8s.io/v1beta1:RoleBindingList",
"kubernetes:rbac.authorization.k8s.io/v1beta1:RoleList",
"kubernetes:resource.k8s.io/v1alpha1:PodSchedulingList",
"kubernetes:resource.k8s.io/v1alpha1:ResourceClaimList",
"kubernetes:resource.k8s.io/v1alpha1:ResourceClaimTemplateList",
"kubernetes:resource.k8s.io/v1alpha1:ResourceClassList",
"kubernetes:resource.k8s.io/v1alpha2:PodSchedulingContextList",
"kubernetes:resource.k8s.io/v1alpha2:ResourceClaimList",
"kubernetes:resource.k8s.io/v1alpha2:ResourceClaimTemplateList",
"kubernetes:resource.k8s.io/v1alpha2:ResourceClassList",
"kubernetes:scheduling.k8s.io/v1:PriorityClassList",
"kubernetes:scheduling.k8s.io/v1alpha1:PriorityClassList",
"kubernetes:scheduling.k8s.io/v1beta1:PriorityClassList",
"kubernetes:settings.k8s.io/v1alpha1:PodPresetList",
"kubernetes:storage.k8s.io/v1:CSIDriverList",
"kubernetes:storage.k8s.io/v1:CSINodeList",
"kubernetes:storage.k8s.io/v1:CSIStorageCapacityList",
"kubernetes:storage.k8s.io/v1:StorageClassList",
"kubernetes:storage.k8s.io/v1:VolumeAttachmentList",
"kubernetes:storage.k8s.io/v1alpha1:VolumeAttachmentList",
"kubernetes:storage.k8s.io/v1beta1:CSIDriverList",
"kubernetes:storage.k8s.io/v1beta1:CSINodeList",
"kubernetes:storage.k8s.io/v1beta1:CSIStorageCapacityList",
"kubernetes:storage.k8s.io/v1beta1:StorageClassList",
"kubernetes:storage.k8s.io/v1beta1:VolumeAttachmentList",
)
50 changes: 50 additions & 0 deletions provider/pkg/kinds/kinds_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package kinds

import (
"testing"
)

func TestKind_Namespaced(t *testing.T) {
tests := []struct {
name string
k Kind
wantKnown bool
wantNamespaced bool
}{
{
"Standard kind",
Deployment,
true,
true,
},
{
"List kind",
DeploymentList,
true,
true,
},
{
"Unknown kind",
Kind("TokenReview"),
false,
false,
},
{
"Unknown list kind",
Kind("TokenReviewList"),
false,
false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotKnown, gotNamespaced := tt.k.Namespaced()
if gotKnown != tt.wantKnown {
t.Errorf("Namespaced() gotKnown = %v, want %v", gotKnown, tt.wantKnown)
}
if gotNamespaced != tt.wantNamespaced {
t.Errorf("Namespaced() gotNamespaced = %v, want %v", gotNamespaced, tt.wantNamespaced)
}
})
}
}
12 changes: 12 additions & 0 deletions provider/pkg/provider/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -1255,6 +1255,13 @@ func (k *kubeProvider) Check(ctx context.Context, req *pulumirpc.CheckRequest) (
return k.helmReleaseProvider.Check(ctx, req)
}

if isListURN(urn) {
// TODO: It might be possible to automatically expand List resources into a list of the underlying resources.
// Until then, return a descriptive error message. https://github.com/pulumi/pulumi-kubernetes/issues/2494
return nil, fmt.Errorf("list resources exist for compatibility with YAML manifests and Helm charts, " +
"and cannot be created directly. Use the underlying resource type instead")
}

if !k.serverSideApplyMode && isPatchURN(urn) {
return nil, fmt.Errorf("patch resources require Server-side Apply mode, which is enabled using the " +
"`enableServerSideApply` Provider config")
Expand Down Expand Up @@ -2464,6 +2471,11 @@ func isPatchURN(urn resource.URN) bool {
return kinds.PatchQualifiedTypes.Has(urn.QualifiedType().String())
}

// isListURN returns true if the URN is for a List resource.
func isListURN(urn resource.URN) bool {
return kinds.ListQualifiedTypes.Has(urn.QualifiedType().String())
}

// GetPluginInfo returns generic information about this plugin, like its version.
func (k *kubeProvider) GetPluginInfo(context.Context, *pbempty.Empty) (*pulumirpc.PluginInfo, error) {
return &pulumirpc.PluginInfo{
Expand Down
38 changes: 38 additions & 0 deletions provider/pkg/provider/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,3 +191,41 @@ func Test_isPatchURN(t *testing.T) {
})
}
}

func Test_isListURN(t *testing.T) {
type args struct {
urn resource.URN
}
tests := []struct {
name string
args args
want bool
}{
{
name: "list URN",
args: args{
urn: resource.NewURN("test", "test", "", "kubernetes:apps/v1:DeploymentList", "test"),
},
want: true,
},
{
name: "regular URN",
args: args{
urn: resource.NewURN("test", "test", "", "kubernetes:apps/v1:Deployment", "test"),
},
want: false,
},
{
name: "CustomResource with List suffix",
args: args{
urn: resource.NewURN("test", "test", "", "kubernetes:example/v1alpha1:ExampleList", "test"),
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equalf(t, tt.want, isListURN(tt.args.urn), "isListURN(%v)", tt.args.urn)
})
}
}

0 comments on commit a160f74

Please sign in to comment.