From eb94878c0a50745dee1484069f9b0e244d272431 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Fri, 19 Jan 2024 14:21:32 -0500 Subject: [PATCH] Fix codegen to use correct nil-vs-empty semantics in fake clients On most errors, the real clients return an empty object, whereas the fake clients were returning nil; this meant that code that checked for nil would appear to be working in unit tests, but then fail against real data. --- .../fake/generator_fake_for_type.go | 95 +++++++++++-------- 1 file changed, 54 insertions(+), 41 deletions(-) diff --git a/staging/src/k8s.io/code-generator/cmd/client-gen/generators/fake/generator_fake_for_type.go b/staging/src/k8s.io/code-generator/cmd/client-gen/generators/fake/generator_fake_for_type.go index 391461604fb..01292bf7e09 100644 --- a/staging/src/k8s.io/code-generator/cmd/client-gen/generators/fake/generator_fake_for_type.go +++ b/staging/src/k8s.io/code-generator/cmd/client-gen/generators/fake/generator_fake_for_type.go @@ -334,11 +334,12 @@ var $.type|allLowercasePlural$Kind = $.SchemeGroupVersion|raw$.WithKind("$.type| var listTemplate = ` // List takes label and field selectors, and returns the list of $.type|publicPlural$ that match those selectors. func (c *Fake$.type|publicPlural$) List(ctx context.Context, opts $.ListOptions|raw$) (result *$.type|raw$List, err error) { + emptyResult := &$.type|raw$List{} obj, err := c.Fake. - $if .namespaced$Invokes($.NewListAction|raw$($.type|allLowercasePlural$Resource, $.type|allLowercasePlural$Kind, c.ns, opts), &$.type|raw$List{}) - $else$Invokes($.NewRootListAction|raw$($.type|allLowercasePlural$Resource, $.type|allLowercasePlural$Kind, opts), &$.type|raw$List{})$end$ + $if .namespaced$Invokes($.NewListAction|raw$($.type|allLowercasePlural$Resource, $.type|allLowercasePlural$Kind, c.ns, opts), emptyResult) + $else$Invokes($.NewRootListAction|raw$($.type|allLowercasePlural$Resource, $.type|allLowercasePlural$Kind, opts), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.type|raw$List), err } @@ -347,11 +348,12 @@ func (c *Fake$.type|publicPlural$) List(ctx context.Context, opts $.ListOptions| var listUsingOptionsTemplate = ` // List takes label and field selectors, and returns the list of $.type|publicPlural$ that match those selectors. func (c *Fake$.type|publicPlural$) List(ctx context.Context, opts $.ListOptions|raw$) (result *$.type|raw$List, err error) { + emptyResult := &$.type|raw$List{} obj, err := c.Fake. - $if .namespaced$Invokes($.NewListAction|raw$($.type|allLowercasePlural$Resource, $.type|allLowercasePlural$Kind, c.ns, opts), &$.type|raw$List{}) - $else$Invokes($.NewRootListAction|raw$($.type|allLowercasePlural$Resource, $.type|allLowercasePlural$Kind, opts), &$.type|raw$List{})$end$ + $if .namespaced$Invokes($.NewListAction|raw$($.type|allLowercasePlural$Resource, $.type|allLowercasePlural$Kind, c.ns, opts), emptyResult) + $else$Invokes($.NewRootListAction|raw$($.type|allLowercasePlural$Resource, $.type|allLowercasePlural$Kind, opts), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } label, _, _ := $.ExtractFromListOptions|raw$(opts) @@ -371,11 +373,12 @@ func (c *Fake$.type|publicPlural$) List(ctx context.Context, opts $.ListOptions| var getTemplate = ` // Get takes name of the $.type|private$, and returns the corresponding $.resultType|private$ object, and an error if there is any. func (c *Fake$.type|publicPlural$) Get(ctx context.Context, name string, options $.GetOptions|raw$) (result *$.resultType|raw$, err error) { + emptyResult := &$.resultType|raw${} obj, err := c.Fake. - $if .namespaced$Invokes($.NewGetAction|raw$($.type|allLowercasePlural$Resource, c.ns, name), &$.resultType|raw${}) - $else$Invokes($.NewRootGetAction|raw$($.type|allLowercasePlural$Resource, name), &$.resultType|raw${})$end$ + $if .namespaced$Invokes($.NewGetAction|raw$($.type|allLowercasePlural$Resource, c.ns, name), emptyResult) + $else$Invokes($.NewRootGetAction|raw$($.type|allLowercasePlural$Resource, name), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.resultType|raw$), err } @@ -384,11 +387,12 @@ func (c *Fake$.type|publicPlural$) Get(ctx context.Context, name string, options var getSubresourceTemplate = ` // Get takes name of the $.type|private$, and returns the corresponding $.resultType|private$ object, and an error if there is any. func (c *Fake$.type|publicPlural$) Get(ctx context.Context, $.type|private$Name string, options $.GetOptions|raw$) (result *$.resultType|raw$, err error) { + emptyResult := &$.resultType|raw${} obj, err := c.Fake. - $if .namespaced$Invokes($.NewGetSubresourceAction|raw$($.type|allLowercasePlural$Resource, c.ns, "$.subresourcePath$", $.type|private$Name), &$.resultType|raw${}) - $else$Invokes($.NewRootGetSubresourceAction|raw$($.type|allLowercasePlural$Resource, "$.subresourcePath$", $.type|private$Name), &$.resultType|raw${})$end$ + $if .namespaced$Invokes($.NewGetSubresourceAction|raw$($.type|allLowercasePlural$Resource, c.ns, "$.subresourcePath$", $.type|private$Name), emptyResult) + $else$Invokes($.NewRootGetSubresourceAction|raw$($.type|allLowercasePlural$Resource, "$.subresourcePath$", $.type|private$Name), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.resultType|raw$), err } @@ -417,11 +421,12 @@ func (c *Fake$.type|publicPlural$) DeleteCollection(ctx context.Context, opts $. var createTemplate = ` // Create takes the representation of a $.inputType|private$ and creates it. Returns the server's representation of the $.resultType|private$, and an error, if there is any. func (c *Fake$.type|publicPlural$) Create(ctx context.Context, $.inputType|private$ *$.inputType|raw$, opts $.CreateOptions|raw$) (result *$.resultType|raw$, err error) { + emptyResult := &$.resultType|raw${} obj, err := c.Fake. - $if .namespaced$Invokes($.NewCreateAction|raw$($.inputType|allLowercasePlural$Resource, c.ns, $.inputType|private$), &$.resultType|raw${}) - $else$Invokes($.NewRootCreateAction|raw$($.inputType|allLowercasePlural$Resource, $.inputType|private$), &$.resultType|raw${})$end$ + $if .namespaced$Invokes($.NewCreateAction|raw$($.inputType|allLowercasePlural$Resource, c.ns, $.inputType|private$), emptyResult) + $else$Invokes($.NewRootCreateAction|raw$($.inputType|allLowercasePlural$Resource, $.inputType|private$), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.resultType|raw$), err } @@ -430,11 +435,12 @@ func (c *Fake$.type|publicPlural$) Create(ctx context.Context, $.inputType|priva var createSubresourceTemplate = ` // Create takes the representation of a $.inputType|private$ and creates it. Returns the server's representation of the $.resultType|private$, and an error, if there is any. func (c *Fake$.type|publicPlural$) Create(ctx context.Context, $.type|private$Name string, $.inputType|private$ *$.inputType|raw$, opts $.CreateOptions|raw$) (result *$.resultType|raw$, err error) { + emptyResult := &$.resultType|raw${} obj, err := c.Fake. - $if .namespaced$Invokes($.NewCreateSubresourceAction|raw$($.type|allLowercasePlural$Resource, $.type|private$Name, "$.subresourcePath$", c.ns, $.inputType|private$), &$.resultType|raw${}) - $else$Invokes($.NewRootCreateSubresourceAction|raw$($.type|allLowercasePlural$Resource, $.type|private$Name, "$.subresourcePath$", $.inputType|private$), &$.resultType|raw${})$end$ + $if .namespaced$Invokes($.NewCreateSubresourceAction|raw$($.type|allLowercasePlural$Resource, $.type|private$Name, "$.subresourcePath$", c.ns, $.inputType|private$), emptyResult) + $else$Invokes($.NewRootCreateSubresourceAction|raw$($.type|allLowercasePlural$Resource, $.type|private$Name, "$.subresourcePath$", $.inputType|private$), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.resultType|raw$), err } @@ -443,11 +449,12 @@ func (c *Fake$.type|publicPlural$) Create(ctx context.Context, $.type|private$Na var updateTemplate = ` // Update takes the representation of a $.inputType|private$ and updates it. Returns the server's representation of the $.resultType|private$, and an error, if there is any. func (c *Fake$.type|publicPlural$) Update(ctx context.Context, $.inputType|private$ *$.inputType|raw$, opts $.UpdateOptions|raw$) (result *$.resultType|raw$, err error) { + emptyResult := &$.resultType|raw${} obj, err := c.Fake. - $if .namespaced$Invokes($.NewUpdateAction|raw$($.inputType|allLowercasePlural$Resource, c.ns, $.inputType|private$), &$.resultType|raw${}) - $else$Invokes($.NewRootUpdateAction|raw$($.inputType|allLowercasePlural$Resource, $.inputType|private$), &$.resultType|raw${})$end$ + $if .namespaced$Invokes($.NewUpdateAction|raw$($.inputType|allLowercasePlural$Resource, c.ns, $.inputType|private$), emptyResult) + $else$Invokes($.NewRootUpdateAction|raw$($.inputType|allLowercasePlural$Resource, $.inputType|private$), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.resultType|raw$), err } @@ -456,11 +463,12 @@ func (c *Fake$.type|publicPlural$) Update(ctx context.Context, $.inputType|priva var updateSubresourceTemplate = ` // Update takes the representation of a $.inputType|private$ and updates it. Returns the server's representation of the $.resultType|private$, and an error, if there is any. func (c *Fake$.type|publicPlural$) Update(ctx context.Context, $.type|private$Name string, $.inputType|private$ *$.inputType|raw$, opts $.UpdateOptions|raw$) (result *$.resultType|raw$, err error) { + emptyResult := &$.resultType|raw${} obj, err := c.Fake. $if .namespaced$Invokes($.NewUpdateSubresourceAction|raw$($.type|allLowercasePlural$Resource, "$.subresourcePath$", c.ns, $.inputType|private$), &$.inputType|raw${}) - $else$Invokes($.NewRootUpdateSubresourceAction|raw$($.type|allLowercasePlural$Resource, "$.subresourcePath$", $.inputType|private$), &$.resultType|raw${})$end$ + $else$Invokes($.NewRootUpdateSubresourceAction|raw$($.type|allLowercasePlural$Resource, "$.subresourcePath$", $.inputType|private$), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.resultType|raw$), err } @@ -469,12 +477,13 @@ func (c *Fake$.type|publicPlural$) Update(ctx context.Context, $.type|private$Na var updateStatusTemplate = ` // UpdateStatus was generated because the type contains a Status member. // Add a +genclient:noStatus comment above the type to avoid generating UpdateStatus(). -func (c *Fake$.type|publicPlural$) UpdateStatus(ctx context.Context, $.type|private$ *$.type|raw$, opts $.UpdateOptions|raw$) (*$.type|raw$, error) { +func (c *Fake$.type|publicPlural$) UpdateStatus(ctx context.Context, $.type|private$ *$.type|raw$, opts $.UpdateOptions|raw$) (result *$.type|raw$, err error) { + emptyResult := &$.type|raw${} obj, err := c.Fake. - $if .namespaced$Invokes($.NewUpdateSubresourceAction|raw$($.type|allLowercasePlural$Resource, "status", c.ns, $.type|private$), &$.type|raw${}) - $else$Invokes($.NewRootUpdateSubresourceAction|raw$($.type|allLowercasePlural$Resource, "status", $.type|private$), &$.type|raw${})$end$ + $if .namespaced$Invokes($.NewUpdateSubresourceAction|raw$($.type|allLowercasePlural$Resource, "status", c.ns, $.type|private$), emptyResult) + $else$Invokes($.NewRootUpdateSubresourceAction|raw$($.type|allLowercasePlural$Resource, "status", $.type|private$), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.type|raw$), err } @@ -492,11 +501,12 @@ func (c *Fake$.type|publicPlural$) Watch(ctx context.Context, opts $.ListOptions var patchTemplate = ` // Patch applies the patch and returns the patched $.resultType|private$. func (c *Fake$.type|publicPlural$) Patch(ctx context.Context, name string, pt $.PatchType|raw$, data []byte, opts $.PatchOptions|raw$, subresources ...string) (result *$.resultType|raw$, err error) { + emptyResult := &$.resultType|raw${} obj, err := c.Fake. - $if .namespaced$Invokes($.NewPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, c.ns, name, pt, data, subresources... ), &$.resultType|raw${}) - $else$Invokes($.NewRootPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, name, pt, data, subresources...), &$.resultType|raw${})$end$ + $if .namespaced$Invokes($.NewPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, c.ns, name, pt, data, subresources... ), emptyResult) + $else$Invokes($.NewRootPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, name, pt, data, subresources...), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.resultType|raw$), err } @@ -512,15 +522,16 @@ func (c *Fake$.type|publicPlural$) Apply(ctx context.Context, $.inputType|privat if err != nil { return nil, err } - name := $.inputType|private$.Name + name := $.inputType|private$.Name if name == nil { return nil, fmt.Errorf("$.inputType|private$.Name must be provided to Apply") } + emptyResult := &$.resultType|raw${} obj, err := c.Fake. - $if .namespaced$Invokes($.NewPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, c.ns, *name, $.ApplyPatchType|raw$, data), &$.resultType|raw${}) - $else$Invokes($.NewRootPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, *name, $.ApplyPatchType|raw$, data), &$.resultType|raw${})$end$ + $if .namespaced$Invokes($.NewPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, c.ns, *name, $.ApplyPatchType|raw$, data), emptyResult) + $else$Invokes($.NewRootPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, *name, $.ApplyPatchType|raw$, data), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.resultType|raw$), err } @@ -537,15 +548,16 @@ func (c *Fake$.type|publicPlural$) ApplyStatus(ctx context.Context, $.inputType| if err != nil { return nil, err } - name := $.inputType|private$.Name + name := $.inputType|private$.Name if name == nil { return nil, fmt.Errorf("$.inputType|private$.Name must be provided to Apply") } + emptyResult := &$.resultType|raw${} obj, err := c.Fake. - $if .namespaced$Invokes($.NewPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, c.ns, *name, $.ApplyPatchType|raw$, data, "status"), &$.resultType|raw${}) - $else$Invokes($.NewRootPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, *name, $.ApplyPatchType|raw$, data, "status"), &$.resultType|raw${})$end$ + $if .namespaced$Invokes($.NewPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, c.ns, *name, $.ApplyPatchType|raw$, data, "status"), emptyResult) + $else$Invokes($.NewRootPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, *name, $.ApplyPatchType|raw$, data, "status"), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.resultType|raw$), err } @@ -562,11 +574,12 @@ func (c *Fake$.type|publicPlural$) Apply(ctx context.Context, $.type|private$Nam if err != nil { return nil, err } + emptyResult := &$.resultType|raw${} obj, err := c.Fake. - $if .namespaced$Invokes($.NewPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, c.ns, $.type|private$Name, $.ApplyPatchType|raw$, data, "status"), &$.resultType|raw${}) - $else$Invokes($.NewRootPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, $.type|private$Name, $.ApplyPatchType|raw$, data, "status"), &$.resultType|raw${})$end$ + $if .namespaced$Invokes($.NewPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, c.ns, $.type|private$Name, $.ApplyPatchType|raw$, data, "status"), emptyResult) + $else$Invokes($.NewRootPatchSubresourceAction|raw$($.type|allLowercasePlural$Resource, $.type|private$Name, $.ApplyPatchType|raw$, data, "status"), emptyResult)$end$ if obj == nil { - return nil, err + return emptyResult, err } return obj.(*$.resultType|raw$), err }