fix predeclared linter

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>
This commit is contained in:
Tim Ramlot 2024-04-29 17:32:49 +02:00
parent 000e9ff4c9
commit 24e47ff364
No known key found for this signature in database
GPG Key ID: 47428728E0C2878D
12 changed files with 98 additions and 103 deletions

View File

@ -12,7 +12,6 @@ issues:
- nilerr
- forbidigo
- interfacebloat
- predeclared
- noctx
- nilnil
- nakedret

View File

@ -27,15 +27,15 @@ import (
)
func ValidateChallengeUpdate(a *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) (field.ErrorList, []string) {
old, ok := oldObj.(*cmacme.Challenge)
new := newObj.(*cmacme.Challenge)
oldChallenge, ok := oldObj.(*cmacme.Challenge)
newChallenge := newObj.(*cmacme.Challenge)
// if oldObj is not set, the Update operation is always valid.
if !ok || old == nil {
if !ok || oldChallenge == nil {
return nil, nil
}
el := field.ErrorList{}
if !reflect.DeepEqual(old.Spec, new.Spec) {
if !reflect.DeepEqual(oldChallenge.Spec, newChallenge.Spec) {
el = append(el, field.Forbidden(field.NewPath("spec"), "challenge spec is immutable after creation"))
}
return el, nil

View File

@ -27,16 +27,16 @@ import (
)
func ValidateOrderUpdate(a *admissionv1.AdmissionRequest, oldObj, newObj runtime.Object) (field.ErrorList, []string) {
old, ok := oldObj.(*cmacme.Order)
new := newObj.(*cmacme.Order)
oldOrder, ok := oldObj.(*cmacme.Order)
newOrder := newObj.(*cmacme.Order)
// if oldObj is not set, the Update operation is always valid.
if !ok || old == nil {
if !ok || oldOrder == nil {
return nil, nil
}
el := field.ErrorList{}
el = append(el, ValidateOrderSpecUpdate(old.Spec, new.Spec, field.NewPath("spec"))...)
el = append(el, ValidateOrderStatusUpdate(old.Status, new.Status, field.NewPath("status"))...)
el = append(el, ValidateOrderSpecUpdate(oldOrder.Spec, newOrder.Spec, field.NewPath("spec"))...)
el = append(el, ValidateOrderStatusUpdate(oldOrder.Status, newOrder.Status, field.NewPath("status"))...)
return el, nil
}
@ -44,35 +44,35 @@ func ValidateOrder(a *admissionv1.AdmissionRequest, obj runtime.Object) (field.E
return nil, nil
}
func ValidateOrderSpecUpdate(old, new cmacme.OrderSpec, fldPath *field.Path) field.ErrorList {
func ValidateOrderSpecUpdate(oldOrder, newOrder cmacme.OrderSpec, fldPath *field.Path) field.ErrorList {
el := field.ErrorList{}
if len(old.Request) > 0 && !bytes.Equal(old.Request, new.Request) {
if len(oldOrder.Request) > 0 && !bytes.Equal(oldOrder.Request, newOrder.Request) {
el = append(el, field.Forbidden(fldPath.Child("request"), "field is immutable once set"))
}
return el
}
func ValidateOrderStatusUpdate(old, new cmacme.OrderStatus, fldPath *field.Path) field.ErrorList {
func ValidateOrderStatusUpdate(oldStatus, newStatus cmacme.OrderStatus, fldPath *field.Path) field.ErrorList {
el := field.ErrorList{}
// once the order URL has been set, it cannot be changed
if old.URL != "" && old.URL != new.URL {
if oldStatus.URL != "" && oldStatus.URL != newStatus.URL {
el = append(el, field.Forbidden(fldPath.Child("url"), "field is immutable once set"))
}
// once the FinalizeURL has been set, it cannot be changed
if old.FinalizeURL != "" && old.FinalizeURL != new.FinalizeURL {
if oldStatus.FinalizeURL != "" && oldStatus.FinalizeURL != newStatus.FinalizeURL {
el = append(el, field.Forbidden(fldPath.Child("finalizeURL"), "field is immutable once set"))
}
// once the Certificate has been issued, it cannot be changed
if len(old.Certificate) > 0 && !bytes.Equal(old.Certificate, new.Certificate) {
if len(oldStatus.Certificate) > 0 && !bytes.Equal(oldStatus.Certificate, newStatus.Certificate) {
el = append(el, field.Forbidden(fldPath.Child("certificate"), "field is immutable once set"))
}
if len(old.Authorizations) > 0 {
if len(oldStatus.Authorizations) > 0 {
fldPath := fldPath.Child("authorizations")
// once at least one Authorization has been inserted, no more can be added
// or deleted from the Order
if len(old.Authorizations) != len(new.Authorizations) {
if len(oldStatus.Authorizations) != len(newStatus.Authorizations) {
el = append(el, field.Forbidden(fldPath, "field is immutable once set"))
}
@ -80,43 +80,43 @@ func ValidateOrderStatusUpdate(old, new cmacme.OrderStatus, fldPath *field.Path)
// the updates that the user requested on each Authorization.
// fields on Authorization's cannot be changed after being set from
// their zero value.
for i := range old.Authorizations {
for i := range oldStatus.Authorizations {
fldPath := fldPath.Index(i)
old := old.Authorizations[i]
new := new.Authorizations[i]
if old.URL != "" && old.URL != new.URL {
oldAuthz := oldStatus.Authorizations[i]
newAuthz := newStatus.Authorizations[i]
if oldAuthz.URL != "" && oldAuthz.URL != newAuthz.URL {
el = append(el, field.Forbidden(fldPath.Child("url"), "field is immutable once set"))
}
if old.Identifier != "" && old.Identifier != new.Identifier {
if oldAuthz.Identifier != "" && oldAuthz.Identifier != newAuthz.Identifier {
el = append(el, field.Forbidden(fldPath.Child("identifier"), "field is immutable once set"))
}
// don't allow the value of the Wildcard field to change unless the
// old value is nil
if old.Wildcard != nil && (new.Wildcard == nil || *old.Wildcard != *new.Wildcard) {
if oldAuthz.Wildcard != nil && (newAuthz.Wildcard == nil || *oldAuthz.Wildcard != *newAuthz.Wildcard) {
el = append(el, field.Forbidden(fldPath.Child("wildcard"), "field is immutable once set"))
}
if old.InitialState != "" && (old.InitialState != new.InitialState) {
if oldAuthz.InitialState != "" && (oldAuthz.InitialState != newAuthz.InitialState) {
el = append(el, field.Forbidden(fldPath.Child("initialState"), "field is immutable once set"))
}
if len(old.Challenges) > 0 {
if len(oldAuthz.Challenges) > 0 {
fldPath := fldPath.Child("challenges")
if len(old.Challenges) != len(new.Challenges) {
if len(oldAuthz.Challenges) != len(newAuthz.Challenges) {
el = append(el, field.Forbidden(fldPath, "field is immutable once set"))
}
for i := range old.Challenges {
for i := range oldAuthz.Challenges {
fldPath := fldPath.Index(i)
old := old.Challenges[i]
new := new.Challenges[i]
oldChallenge := oldAuthz.Challenges[i]
newChallenge := newAuthz.Challenges[i]
if old.URL != "" && old.URL != new.URL {
if oldChallenge.URL != "" && oldChallenge.URL != newChallenge.URL {
el = append(el, field.Forbidden(fldPath.Child("url"), "field is immutable once set"))
}
if old.Type != "" && old.Type != new.Type {
if oldChallenge.Type != "" && oldChallenge.Type != newChallenge.Type {
el = append(el, field.Forbidden(fldPath.Child("type"), "field is immutable once set"))
}
if old.Token != "" && old.Token != new.Token {
if oldChallenge.Token != "" && oldChallenge.Token != newChallenge.Token {
el = append(el, field.Forbidden(fldPath.Child("token"), "field is immutable once set"))
}
}

View File

@ -55,11 +55,11 @@ func testImmutableOrderField(t *testing.T, fldPath *field.Path, setter func(*cma
field.Forbidden(fldPath, "field is immutable once set"),
}
var expectedWarnings []string
old := &cmacme.Order{}
new := &cmacme.Order{}
setter(old, testValueOptionOne)
setter(new, testValueOptionTwo)
errs, warnings := ValidateOrderUpdate(someAdmissionRequest, old, new)
oldOrder := &cmacme.Order{}
newOrder := &cmacme.Order{}
setter(oldOrder, testValueOptionOne)
setter(newOrder, testValueOptionTwo)
errs, warnings := ValidateOrderUpdate(someAdmissionRequest, oldOrder, newOrder)
if len(errs) != len(expectedErrs) {
t.Errorf("Expected errors %v but got %v", expectedErrs, errs)
return
@ -77,11 +77,11 @@ func testImmutableOrderField(t *testing.T, fldPath *field.Path, setter func(*cma
t.Run("should allow updates to "+fldPath.String()+" if not already set", func(t *testing.T) {
expectedErrs := []*field.Error{}
var expectedWarnings []string
old := &cmacme.Order{}
new := &cmacme.Order{}
setter(old, testValueNone)
setter(new, testValueOptionOne)
errs, warnings := ValidateOrderUpdate(someAdmissionRequest, old, new)
oldOrder := &cmacme.Order{}
newOrder := &cmacme.Order{}
setter(oldOrder, testValueNone)
setter(newOrder, testValueOptionOne)
errs, warnings := ValidateOrderUpdate(someAdmissionRequest, oldOrder, newOrder)
if len(errs) != len(expectedErrs) {
t.Errorf("Expected errors %v but got %v", expectedErrs, errs)
return

View File

@ -1159,7 +1159,7 @@ func TestNewConfig(t *testing.T) {
}),
),
expectedErr: nil,
checkFunc: func(cfg *vault.Config, error error) error {
checkFunc: func(cfg *vault.Config, err error) error {
testCA := x509.NewCertPool()
testCA.AppendCertsFromPEM([]byte(testLeafCertificate))
clientCA := cfg.HttpClient.Transport.(*http.Transport).TLSClientConfig.RootCAs
@ -1185,9 +1185,9 @@ func TestNewConfig(t *testing.T) {
},
},
)),
checkFunc: func(cfg *vault.Config, error error) error {
if error != nil {
return error
checkFunc: func(cfg *vault.Config, err error) error {
if err != nil {
return err
}
testCA := x509.NewCertPool()
@ -1214,9 +1214,9 @@ func TestNewConfig(t *testing.T) {
},
},
)),
checkFunc: func(cfg *vault.Config, error error) error {
if error != nil {
return error
checkFunc: func(cfg *vault.Config, err error) error {
if err != nil {
return err
}
testCA := x509.NewCertPool()
@ -1291,9 +1291,9 @@ func TestNewConfig(t *testing.T) {
},
},
)),
checkFunc: func(cfg *vault.Config, error error) error {
if error != nil {
return error
checkFunc: func(cfg *vault.Config, err error) error {
if err != nil {
return err
}
certificates := cfg.HttpClient.Transport.(*http.Transport).TLSClientConfig.Certificates

View File

@ -72,10 +72,10 @@ func newObjectUpdater(cl versioned.Interface, fieldManager string) objectUpdater
// Only the Finalizers and Status fields may be modified. If there are any
// modifications to new object, outside of the Finalizers and Status fields,
// this function return an error.
func (o *defaultObjectUpdater) updateObject(ctx context.Context, old, new *cmacme.Challenge) error {
func (o *defaultObjectUpdater) updateObject(ctx context.Context, oldChallenge, newChallenge *cmacme.Challenge) error {
if !apiequality.Semantic.DeepEqual(
gen.ChallengeFrom(old, gen.SetChallengeFinalizers(nil), gen.ResetChallengeStatus()),
gen.ChallengeFrom(new, gen.SetChallengeFinalizers(nil), gen.ResetChallengeStatus()),
gen.ChallengeFrom(oldChallenge, gen.SetChallengeFinalizers(nil), gen.ResetChallengeStatus()),
gen.ChallengeFrom(newChallenge, gen.SetChallengeFinalizers(nil), gen.ResetChallengeStatus()),
) {
return fmt.Errorf(
"%w: in updateObject: unexpected differences between old and new: only the finalizers and status fields may be modified",
@ -84,11 +84,11 @@ func (o *defaultObjectUpdater) updateObject(ctx context.Context, old, new *cmacm
}
var updateFunctions []func() (*cmacme.Challenge, error)
if !apiequality.Semantic.DeepEqual(old.Status, new.Status) {
if !apiequality.Semantic.DeepEqual(oldChallenge.Status, newChallenge.Status) {
updateFunctions = append(
updateFunctions,
func() (*cmacme.Challenge, error) {
if obj, err := o.updateStatus(ctx, new); err != nil {
if obj, err := o.updateStatus(ctx, newChallenge); err != nil {
return obj, fmt.Errorf("when updating the status: %w", err)
} else {
return obj, nil
@ -96,11 +96,11 @@ func (o *defaultObjectUpdater) updateObject(ctx context.Context, old, new *cmacm
},
)
}
if !apiequality.Semantic.DeepEqual(old.Finalizers, new.Finalizers) {
if !apiequality.Semantic.DeepEqual(oldChallenge.Finalizers, newChallenge.Finalizers) {
updateFunctions = append(
updateFunctions,
func() (*cmacme.Challenge, error) {
if obj, err := o.update(ctx, new); err != nil {
if obj, err := o.update(ctx, newChallenge); err != nil {
return obj, fmt.Errorf("when updating the finalizers: %w", err)
} else {
return obj, nil
@ -116,7 +116,7 @@ func (o *defaultObjectUpdater) updateObject(ctx context.Context, old, new *cmacm
return nil
}
} else {
new = o
newChallenge = o
}
}
return utilerrors.NewAggregate(errors)
@ -126,12 +126,12 @@ type objectUpdateClientDefault struct {
cl versioned.Interface
}
func (o *objectUpdateClientDefault) update(ctx context.Context, new *cmacme.Challenge) (*cmacme.Challenge, error) {
return o.cl.AcmeV1().Challenges(new.Namespace).Update(ctx, new, metav1.UpdateOptions{})
func (o *objectUpdateClientDefault) update(ctx context.Context, challenge *cmacme.Challenge) (*cmacme.Challenge, error) {
return o.cl.AcmeV1().Challenges(challenge.Namespace).Update(ctx, challenge, metav1.UpdateOptions{})
}
func (o *objectUpdateClientDefault) updateStatus(ctx context.Context, new *cmacme.Challenge) (*cmacme.Challenge, error) {
return o.cl.AcmeV1().Challenges(new.Namespace).UpdateStatus(ctx, new, metav1.UpdateOptions{})
func (o *objectUpdateClientDefault) updateStatus(ctx context.Context, challenge *cmacme.Challenge) (*cmacme.Challenge, error) {
return o.cl.AcmeV1().Challenges(challenge.Namespace).UpdateStatus(ctx, challenge, metav1.UpdateOptions{})
}
type objectUpdateClientSSA struct {
@ -139,10 +139,10 @@ type objectUpdateClientSSA struct {
fieldManager string
}
func (o *objectUpdateClientSSA) update(ctx context.Context, new *cmacme.Challenge) (*cmacme.Challenge, error) {
return internalchallenges.Apply(ctx, o.cl, o.fieldManager, new)
func (o *objectUpdateClientSSA) update(ctx context.Context, challenge *cmacme.Challenge) (*cmacme.Challenge, error) {
return internalchallenges.Apply(ctx, o.cl, o.fieldManager, challenge)
}
func (o *objectUpdateClientSSA) updateStatus(ctx context.Context, new *cmacme.Challenge) (*cmacme.Challenge, error) {
return internalchallenges.ApplyStatus(ctx, o.cl, o.fieldManager, new)
func (o *objectUpdateClientSSA) updateStatus(ctx context.Context, challenge *cmacme.Challenge) (*cmacme.Challenge, error) {
return internalchallenges.ApplyStatus(ctx, o.cl, o.fieldManager, challenge)
}

View File

@ -129,9 +129,9 @@ func runUpdateObjectTests(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.TODO()
old := gen.Challenge("c1")
new := gen.ChallengeFrom(old, tt.mods...)
objects := []runtime.Object{old}
oldChallenge := gen.Challenge("c1")
newChallenge := gen.ChallengeFrom(oldChallenge, tt.mods...)
objects := []runtime.Object{oldChallenge}
if tt.notFound {
t.Log("Simulating a situation where the target object has been deleted")
objects = nil
@ -151,7 +151,7 @@ func runUpdateObjectTests(t *testing.T) {
}
updater := newObjectUpdater(cl, "test-fieldmanager")
t.Log("Calling updateObject")
updateObjectErr := updater.updateObject(ctx, old, new)
updateObjectErr := updater.updateObject(ctx, oldChallenge, newChallenge)
if tt.errorMessage == "" {
assert.NoError(t, updateObjectErr)
} else {
@ -164,16 +164,16 @@ func runUpdateObjectTests(t *testing.T) {
if !tt.notFound {
t.Log("Checking whether the object was updated")
actual, err := cl.AcmeV1().Challenges(old.Namespace).Get(ctx, old.Name, metav1.GetOptions{})
actual, err := cl.AcmeV1().Challenges(oldChallenge.Namespace).Get(ctx, oldChallenge.Name, metav1.GetOptions{})
require.NoError(t, err)
if updateObjectErr == nil {
assert.Equal(t, new, actual, "updateObject did not return an error so the object in the API should have been updated")
assert.Equal(t, newChallenge, actual, "updateObject did not return an error so the object in the API should have been updated")
} else {
if !errors.Is(updateObjectErr, simulatedUpdateError) {
assert.Equal(t, new.Finalizers, actual.Finalizers, "The Update did not fail so the Finalizers of the API object should have been updated")
assert.Equal(t, newChallenge.Finalizers, actual.Finalizers, "The Update did not fail so the Finalizers of the API object should have been updated")
}
if !errors.Is(updateObjectErr, simulatedUpdateStatusError) {
assert.Equal(t, new.Status, actual.Status, "The UpdateStatus did not fail so the Status of the API object should have been updated")
assert.Equal(t, newChallenge.Status, actual.Status, "The UpdateStatus did not fail so the Status of the API object should have been updated")
}
}
}

View File

@ -297,11 +297,7 @@ func buildCertificates(
cmLister cmlisters.CertificateLister,
ingLike metav1.Object,
issuerName, issuerKind, issuerGroup string,
) (new, update []*cmapi.Certificate, _ error) {
var newCrts []*cmapi.Certificate
var updateCrts []*cmapi.Certificate
) (newCrts, updateCrts []*cmapi.Certificate, _ error) {
tlsHosts := make(map[corev1.ObjectReference][]string)
switch ingLike := ingLike.(type) {
case *networkingv1.Ingress:

View File

@ -168,21 +168,21 @@ func (c *Controller) Sync(ctx context.Context, cr *cmapi.CertificateRequest) (er
return nil
}
func (c *Controller) updateCertificateRequestStatusAndAnnotations(ctx context.Context, old, new *cmapi.CertificateRequest) error {
func (c *Controller) updateCertificateRequestStatusAndAnnotations(ctx context.Context, oldCR, newCR *cmapi.CertificateRequest) error {
log := logf.FromContext(ctx, "updateStatus")
// if annotations changed we have to call .Update() and not .UpdateStatus()
if !reflect.DeepEqual(old.Annotations, new.Annotations) {
log.V(logf.DebugLevel).Info("updating resource due to change in annotations", "diff", pretty.Diff(old.Annotations, new.Annotations))
return c.updateOrApply(ctx, new)
if !reflect.DeepEqual(oldCR.Annotations, newCR.Annotations) {
log.V(logf.DebugLevel).Info("updating resource due to change in annotations", "diff", pretty.Diff(oldCR.Annotations, newCR.Annotations))
return c.updateOrApply(ctx, newCR)
}
if apiequality.Semantic.DeepEqual(old.Status, new.Status) {
if apiequality.Semantic.DeepEqual(oldCR.Status, newCR.Status) {
return nil
}
log.V(logf.DebugLevel).Info("updating resource due to change in status", "diff", pretty.Diff(old.Status, new.Status))
return c.updateStatusOrApply(ctx, new)
log.V(logf.DebugLevel).Info("updating resource due to change in status", "diff", pretty.Diff(oldCR.Status, newCR.Status))
return c.updateStatusOrApply(ctx, newCR)
}
func (c *Controller) updateOrApply(ctx context.Context, cr *cmapi.CertificateRequest) error {

View File

@ -67,14 +67,14 @@ func (c *controller) Sync(ctx context.Context, iss *cmapi.ClusterIssuer) (err er
return nil
}
func (c *controller) updateIssuerStatus(ctx context.Context, old, new *cmapi.ClusterIssuer) error {
if apiequality.Semantic.DeepEqual(old.Status, new.Status) {
func (c *controller) updateIssuerStatus(ctx context.Context, oldIssuer, newIssuer *cmapi.ClusterIssuer) error {
if apiequality.Semantic.DeepEqual(oldIssuer.Status, newIssuer.Status) {
return nil
}
if utilfeature.DefaultFeatureGate.Enabled(feature.ServerSideApply) {
return internalissuers.ApplyClusterIssuerStatus(ctx, c.cmClient, c.fieldManager, new)
return internalissuers.ApplyClusterIssuerStatus(ctx, c.cmClient, c.fieldManager, newIssuer)
} else {
_, err := c.cmClient.CertmanagerV1().ClusterIssuers().UpdateStatus(ctx, new, metav1.UpdateOptions{})
_, err := c.cmClient.CertmanagerV1().ClusterIssuers().UpdateStatus(ctx, newIssuer, metav1.UpdateOptions{})
return err
}
}

View File

@ -67,15 +67,15 @@ func (c *controller) Sync(ctx context.Context, iss *cmapi.Issuer) (err error) {
return nil
}
func (c *controller) updateIssuerStatus(ctx context.Context, old, new *cmapi.Issuer) error {
if apiequality.Semantic.DeepEqual(old.Status, new.Status) {
func (c *controller) updateIssuerStatus(ctx context.Context, oldIssuer, newIssuer *cmapi.Issuer) error {
if apiequality.Semantic.DeepEqual(oldIssuer.Status, newIssuer.Status) {
return nil
}
if utilfeature.DefaultFeatureGate.Enabled(feature.ServerSideApply) {
return internalissuers.ApplyIssuerStatus(ctx, c.cmClient, c.fieldManager, new)
return internalissuers.ApplyIssuerStatus(ctx, c.cmClient, c.fieldManager, newIssuer)
} else {
_, err := c.cmClient.CertmanagerV1().Issuers(new.Namespace).UpdateStatus(ctx, new, metav1.UpdateOptions{})
_, err := c.cmClient.CertmanagerV1().Issuers(newIssuer.Namespace).UpdateStatus(ctx, newIssuer, metav1.UpdateOptions{})
return err
}
}

View File

@ -120,11 +120,11 @@ func (q *QueuingEventHandler) OnAdd(obj interface{}, isInInitialList bool) {
}
// OnUpdate adds an updated object to the workqueue.
func (q *QueuingEventHandler) OnUpdate(old, new interface{}) {
if reflect.DeepEqual(old, new) {
func (q *QueuingEventHandler) OnUpdate(oldObj, newObj interface{}) {
if reflect.DeepEqual(oldObj, newObj) {
return
}
q.Enqueue(new)
q.Enqueue(newObj)
}
// OnDelete adds a deleted object to the workqueue for processing.
@ -154,11 +154,11 @@ func (b *BlockingEventHandler) OnAdd(obj interface{}, isInInitialList bool) {
}
// OnUpdate synchronously adds an updated object to the workqueue.
func (b *BlockingEventHandler) OnUpdate(old, new interface{}) {
if reflect.DeepEqual(old, new) {
func (b *BlockingEventHandler) OnUpdate(oldObj, newObj interface{}) {
if reflect.DeepEqual(oldObj, newObj) {
return
}
b.WorkFunc(new)
b.WorkFunc(newObj)
}
// OnDelete synchronously adds a deleted object to the workqueue.