From b7f42a0d1f570580419921cdab0c9c77657fdb8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= <charled.breteche@gmail.com> Date: Thu, 28 Apr 2022 14:30:23 +0200 Subject: [PATCH] refactor: remove some api unnecessary pointers (3) (#3707) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: remove some api unnecessary pointers Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com> * refactor: remove some api unnecessary pointers (2) Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com> * refactor: remove some api unnecessary pointers (3) Signed-off-by: Charles-Edouard Brétéché <charled.breteche@gmail.com> --- Makefile | 8 ++---- api/kyverno/v1/image_verification_test.go | 30 ++++++++++---------- api/kyverno/v1/image_verification_types.go | 13 ++++----- api/kyverno/v1/zz_generated.deepcopy.go | 32 ++++++---------------- docs/crd/v1/index.html | 29 ++++++++++++++------ pkg/engine/imageVerify.go | 23 ++++++++-------- pkg/engine/imageVerify_test.go | 6 ++-- pkg/engine/variables/evaluate.go | 4 +-- pkg/engine/variables/vars.go | 8 +++--- 9 files changed, 73 insertions(+), 80 deletions(-) diff --git a/Makefile b/Makefile index b35c7da238..04ceca5cf8 100644 --- a/Makefile +++ b/Makefile @@ -260,9 +260,9 @@ $(GO_ACC): # go-acc merges the result for pks so that it be used by # go tool cover for reporting -test: test-clean test-unit test-e2e +test: test-clean test-unit test-e2e ## Clean tests cache then run unit and e2e tests -test-clean: +test-clean: ## Clean tests cache @echo " cleaning test cache" go clean -testcache ./... @@ -289,9 +289,7 @@ test-cli-test-case-selector-flag: cli test-cli-registry: cli cmd/cli/kubectl-kyverno/kyverno test ./test/cli/registry -# go get downloads and installs the binary -# we temporarily add the GO_ACC to the path -test-unit: $(GO_ACC) +test-unit: $(GO_ACC) ## Run unit tests @echo " running unit tests" go-acc ./... -o $(CODE_COVERAGE_FILE_TXT) diff --git a/api/kyverno/v1/image_verification_test.go b/api/kyverno/v1/image_verification_test.go index 64a2c23914..579e9f3d6d 100644 --- a/api/kyverno/v1/image_verification_test.go +++ b/api/kyverno/v1/image_verification_test.go @@ -89,7 +89,7 @@ func Test_ImageVerification(t *testing.T) { name: "no attestors", subject: ImageVerification{ ImageReferences: []string{"*"}, - Attestors: []*AttestorSet{}, + Attestors: []AttestorSet{}, }, errors: func(i *ImageVerification) field.ErrorList { return field.ErrorList{ @@ -101,13 +101,13 @@ func Test_ImageVerification(t *testing.T) { name: "no entries", subject: ImageVerification{ ImageReferences: []string{"*"}, - Attestors: []*AttestorSet{ - {Entries: []*Attestor{}}, + Attestors: []AttestorSet{ + {Entries: []Attestor{}}, }, }, errors: func(i *ImageVerification) field.ErrorList { return field.ErrorList{ - field.Invalid(path.Child("attestors").Index(0), i.Attestors[0], "An entry is required"), + field.Invalid(path.Child("attestors").Index(0), &i.Attestors[0], "An entry is required"), } }, }, @@ -115,14 +115,14 @@ func Test_ImageVerification(t *testing.T) { name: "empty attestor", subject: ImageVerification{ ImageReferences: []string{"*"}, - Attestors: []*AttestorSet{ - {Entries: []*Attestor{{}}}, + Attestors: []AttestorSet{ + {Entries: []Attestor{{}}}, }, }, errors: func(i *ImageVerification) field.ErrorList { return field.ErrorList{ field.Invalid(path.Child("attestors").Index(0).Child("entries").Index(0), - i.Attestors[0].Entries[0], "One of static key, keyless, or nested attestor is required"), + &i.Attestors[0].Entries[0], "One of static key, keyless, or nested attestor is required"), } }, }, @@ -130,8 +130,8 @@ func Test_ImageVerification(t *testing.T) { name: "empty static key attestor", subject: ImageVerification{ ImageReferences: []string{"*"}, - Attestors: []*AttestorSet{ - {Entries: []*Attestor{{ + Attestors: []AttestorSet{ + {Entries: []Attestor{{ StaticKey: &StaticKeyAttestor{}, }}}, }, @@ -147,8 +147,8 @@ func Test_ImageVerification(t *testing.T) { name: "valid static key attestor", subject: ImageVerification{ ImageReferences: []string{"*"}, - Attestors: []*AttestorSet{ - {Entries: []*Attestor{{ + Attestors: []AttestorSet{ + {Entries: []Attestor{{ StaticKey: &StaticKeyAttestor{Keys: "bla"}, }}}, }, @@ -158,8 +158,8 @@ func Test_ImageVerification(t *testing.T) { name: "invalid keyless attestor", subject: ImageVerification{ ImageReferences: []string{"*"}, - Attestors: []*AttestorSet{ - {Entries: []*Attestor{{ + Attestors: []AttestorSet{ + {Entries: []Attestor{{ Keyless: &KeylessAttestor{Rekor: &CTLog{}, Issuer: "", Subject: ""}, }}}, }, @@ -175,8 +175,8 @@ func Test_ImageVerification(t *testing.T) { name: "valid keyless attestor", subject: ImageVerification{ ImageReferences: []string{"*"}, - Attestors: []*AttestorSet{ - {Entries: []*Attestor{{ + Attestors: []AttestorSet{ + {Entries: []Attestor{{ Keyless: &KeylessAttestor{Rekor: &CTLog{URL: "https://rekor.sigstore.dev"}, Issuer: "bla", Subject: "bla"}, }}}, }, diff --git a/api/kyverno/v1/image_verification_types.go b/api/kyverno/v1/image_verification_types.go index 57ed9d82f2..524b9842db 100644 --- a/api/kyverno/v1/image_verification_types.go +++ b/api/kyverno/v1/image_verification_types.go @@ -48,12 +48,12 @@ type ImageVerification struct { // Attestors specified the required attestors (i.e. authorities) // +kubebuilder:validation:Optional - Attestors []*AttestorSet `json:"attestors,omitempty" yaml:"attestors,omitempty"` + Attestors []AttestorSet `json:"attestors,omitempty" yaml:"attestors,omitempty"` // Attestations are optional checks for signed in-toto Statements used to verify the image. // See https://github.com/in-toto/attestation. Kyverno fetches signed attestations from the // OCI registry and decodes them into a list of Statement declarations. - Attestations []*Attestation `json:"attestations,omitempty" yaml:"attestations,omitempty"` + Attestations []Attestation `json:"attestations,omitempty" yaml:"attestations,omitempty"` // Annotations are used for image verification. // Every specified key-value pair must exist and match in the verified payload. @@ -95,7 +95,7 @@ type AttestorSet struct { // Entries contains the available attestors. An attestor can be a static key, // attributes for keyless verification, or a nested attestor declaration. // +kubebuilder:validation:Optional - Entries []*Attestor `json:"entries,omitempty" yaml:"entries,omitempty"` + Entries []Attestor `json:"entries,omitempty" yaml:"entries,omitempty"` } type Attestor struct { @@ -194,7 +194,7 @@ type Attestation struct { // Conditions are used to verify attributes within a Predicate. If no Conditions are specified // the attestation check is satisfied as long there are predicates that match the predicate type. // +optional - Conditions []*AnyAllConditions `json:"conditions,omitempty" yaml:"conditions,omitempty"` + Conditions []AnyAllConditions `json:"conditions,omitempty" yaml:"conditions,omitempty"` } // Validate implements programmatic validation @@ -332,7 +332,7 @@ func (iv *ImageVerification) Convert() *ImageVerification { copy.ImageReferences = []string{iv.Image} } - attestor := &Attestor{ + attestor := Attestor{ Annotations: iv.Annotations, } @@ -348,9 +348,8 @@ func (iv *ImageVerification) Convert() *ImageVerification { } } - attestorSet := &AttestorSet{} + attestorSet := AttestorSet{} attestorSet.Entries = append(attestorSet.Entries, attestor) - copy.Attestors = append(copy.Attestors, attestorSet) return copy } diff --git a/api/kyverno/v1/zz_generated.deepcopy.go b/api/kyverno/v1/zz_generated.deepcopy.go index 471981f14f..0e9e2d308e 100755 --- a/api/kyverno/v1/zz_generated.deepcopy.go +++ b/api/kyverno/v1/zz_generated.deepcopy.go @@ -91,13 +91,9 @@ func (in *Attestation) DeepCopyInto(out *Attestation) { *out = *in if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions - *out = make([]*AnyAllConditions, len(*in)) + *out = make([]AnyAllConditions, len(*in)) for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(AnyAllConditions) - (*in).DeepCopyInto(*out) - } + (*in)[i].DeepCopyInto(&(*out)[i]) } } } @@ -159,13 +155,9 @@ func (in *AttestorSet) DeepCopyInto(out *AttestorSet) { } if in.Entries != nil { in, out := &in.Entries, &out.Entries - *out = make([]*Attestor, len(*in)) + *out = make([]Attestor, len(*in)) for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(Attestor) - (*in).DeepCopyInto(*out) - } + (*in)[i].DeepCopyInto(&(*out)[i]) } } } @@ -640,24 +632,16 @@ func (in *ImageVerification) DeepCopyInto(out *ImageVerification) { } if in.Attestors != nil { in, out := &in.Attestors, &out.Attestors - *out = make([]*AttestorSet, len(*in)) + *out = make([]AttestorSet, len(*in)) for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(AttestorSet) - (*in).DeepCopyInto(*out) - } + (*in)[i].DeepCopyInto(&(*out)[i]) } } if in.Attestations != nil { in, out := &in.Attestations, &out.Attestations - *out = make([]*Attestation, len(*in)) + *out = make([]Attestation, len(*in)) for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(Attestation) - (*in).DeepCopyInto(*out) - } + (*in)[i].DeepCopyInto(&(*out)[i]) } } if in.Annotations != nil { diff --git a/docs/crd/v1/index.html b/docs/crd/v1/index.html index 3ff978369c..c4699b41b8 100644 --- a/docs/crd/v1/index.html +++ b/docs/crd/v1/index.html @@ -126,6 +126,7 @@ Kubernetes admission/v1.Operation </h3> <p> (<em>Appears on:</em> +<a href="#kyverno.io/v1.Attestation">Attestation</a>, <a href="#kyverno.io/v1.ForEachMutation">ForEachMutation</a>, <a href="#kyverno.io/v1.ForEachValidation">ForEachValidation</a>) </p> @@ -182,6 +183,10 @@ Here, all of the conditions need to pass</p> <h3 id="kyverno.io/v1.Attestation">Attestation </h3> <p> +(<em>Appears on:</em> +<a href="#kyverno.io/v1.ImageVerification">ImageVerification</a>) +</p> +<p> <p>Attestation are checks for signed in-toto Statements that are used to verify the image. See <a href="https://github.com/in-toto/attestation">https://github.com/in-toto/attestation</a>. Kyverno fetches signed attestations from the OCI registry and decodes them into a list of Statements.</p> @@ -209,8 +214,8 @@ string <td> <code>conditions</code></br> <em> -<a href="#kyverno.io/v1.*./api/kyverno/v1.AnyAllConditions"> -[]*./api/kyverno/v1.AnyAllConditions +<a href="#kyverno.io/v1.AnyAllConditions"> +[]AnyAllConditions </a> </em> </td> @@ -226,6 +231,10 @@ the attestation check is satisfied as long there are predicates that match the p <h3 id="kyverno.io/v1.Attestor">Attestor </h3> <p> +(<em>Appears on:</em> +<a href="#kyverno.io/v1.AttestorSet">AttestorSet</a>) +</p> +<p> </p> <table class="table table-striped"> <thead class="thead-dark"> @@ -304,6 +313,10 @@ If specified Repository will override other OCI image repository locations for t <h3 id="kyverno.io/v1.AttestorSet">AttestorSet </h3> <p> +(<em>Appears on:</em> +<a href="#kyverno.io/v1.ImageVerification">ImageVerification</a>) +</p> +<p> </p> <table class="table table-striped"> <thead class="thead-dark"> @@ -330,8 +343,8 @@ value N, then N must be less than or equal to the size of entries, and at least <td> <code>entries</code></br> <em> -<a href="#kyverno.io/v1.*./api/kyverno/v1.Attestor"> -[]*./api/kyverno/v1.Attestor +<a href="#kyverno.io/v1.Attestor"> +[]Attestor </a> </em> </td> @@ -1581,8 +1594,8 @@ Deprecated.</p> <td> <code>attestors</code></br> <em> -<a href="#kyverno.io/v1.*./api/kyverno/v1.AttestorSet"> -[]*./api/kyverno/v1.AttestorSet +<a href="#kyverno.io/v1.AttestorSet"> +[]AttestorSet </a> </em> </td> @@ -1594,8 +1607,8 @@ Deprecated.</p> <td> <code>attestations</code></br> <em> -<a href="#kyverno.io/v1.*./api/kyverno/v1.Attestation"> -[]*./api/kyverno/v1.Attestation +<a href="#kyverno.io/v1.Attestation"> +[]Attestation </a> </em> </td> diff --git a/pkg/engine/imageVerify.go b/pkg/engine/imageVerify.go index cfd81e9ba3..52563606f9 100644 --- a/pkg/engine/imageVerify.go +++ b/pkg/engine/imageVerify.go @@ -312,7 +312,7 @@ func (iv *imageVerifier) verifySignatures(imageVerify *v1.ImageVerification, ima return ruleResponse(*iv.rule, response.ImageVerify, msg, response.RuleStatusPass, nil), digest } -func (iv *imageVerifier) verifyAttestorSet(attestorSet *v1.AttestorSet, imageVerify *v1.ImageVerification, image, path string) (string, error) { +func (iv *imageVerifier) verifyAttestorSet(attestorSet v1.AttestorSet, imageVerify *v1.ImageVerification, image, path string) (string, error) { var errorList []error verifiedCount := 0 attestorSet = expandStaticKeys(attestorSet) @@ -329,7 +329,7 @@ func (iv *imageVerifier) verifyAttestorSet(attestorSet *v1.AttestorSet, imageVer entryError = errors.Wrapf(err, "failed to unmarshal nested attestor %s", attestorPath) } else { attestorPath += ".attestor" - digest, entryError = iv.verifyAttestorSet(nestedAttestorSet, imageVerify, image, attestorPath) + digest, entryError = iv.verifyAttestorSet(*nestedAttestorSet, imageVerify, image, attestorPath) } } else { opts, subPath := iv.buildOptionsAndPath(a, imageVerify, image) @@ -355,8 +355,8 @@ func (iv *imageVerifier) verifyAttestorSet(attestorSet *v1.AttestorSet, imageVer return "", err } -func expandStaticKeys(attestorSet *v1.AttestorSet) *v1.AttestorSet { - var entries []*v1.Attestor +func expandStaticKeys(attestorSet v1.AttestorSet) v1.AttestorSet { + var entries []v1.Attestor for _, e := range attestorSet.Entries { if e.StaticKey != nil { keys := splitPEM(e.StaticKey.Keys) @@ -370,7 +370,7 @@ func expandStaticKeys(attestorSet *v1.AttestorSet) *v1.AttestorSet { entries = append(entries, e) } - return &v1.AttestorSet{ + return v1.AttestorSet{ Count: attestorSet.Count, Entries: entries, } @@ -385,24 +385,23 @@ func splitPEM(pem string) []string { return keys[0 : len(keys)-1] } -func createStaticKeyAttestors(ska *v1.StaticKeyAttestor, keys []string) []*v1.Attestor { - var attestors []*v1.Attestor +func createStaticKeyAttestors(ska *v1.StaticKeyAttestor, keys []string) []v1.Attestor { + var attestors []v1.Attestor for _, k := range keys { - a := &v1.Attestor{ + a := v1.Attestor{ StaticKey: &v1.StaticKeyAttestor{ Keys: k, Intermediates: ska.Intermediates, Roots: ska.Roots, }, } - attestors = append(attestors, a) } return attestors } -func getRequiredCount(as *v1.AttestorSet) int { +func getRequiredCount(as v1.AttestorSet) int { if as.Count == nil || *as.Count == 0 { return len(as.Entries) } @@ -410,7 +409,7 @@ func getRequiredCount(as *v1.AttestorSet) int { return *as.Count } -func (iv *imageVerifier) buildOptionsAndPath(attestor *v1.Attestor, imageVerify *v1.ImageVerification, image string) (*cosign.Options, string) { +func (iv *imageVerifier) buildOptionsAndPath(attestor v1.Attestor, imageVerify *v1.ImageVerification, image string) (*cosign.Options, string) { path := "" opts := &cosign.Options{ ImageRef: image, @@ -519,7 +518,7 @@ func buildStatementMap(statements []map[string]interface{}) map[string][]map[str return results } -func (iv *imageVerifier) checkAttestations(a *v1.Attestation, s map[string]interface{}, img kubeutils.ImageInfo) (bool, error) { +func (iv *imageVerifier) checkAttestations(a v1.Attestation, s map[string]interface{}, img kubeutils.ImageInfo) (bool, error) { if len(a.Conditions) == 0 { return true, nil } diff --git a/pkg/engine/imageVerify_test.go b/pkg/engine/imageVerify_test.go index 5b92e94960..4f5d5a607e 100644 --- a/pkg/engine/imageVerify_test.go +++ b/pkg/engine/imageVerify_test.go @@ -487,9 +487,9 @@ func Test_ExpandKeys(t *testing.T) { assert.Equal(t, 3, len(as.Entries)) } -func createStaticKeyAttestorSet(s string) *kyverno.AttestorSet { - return &kyverno.AttestorSet{ - Entries: []*kyverno.Attestor{ +func createStaticKeyAttestorSet(s string) kyverno.AttestorSet { + return kyverno.AttestorSet{ + Entries: []kyverno.Attestor{ { StaticKey: &kyverno.StaticKeyAttestor{ Keys: s, diff --git a/pkg/engine/variables/evaluate.go b/pkg/engine/variables/evaluate.go index 9960c2331e..51c81df696 100644 --- a/pkg/engine/variables/evaluate.go +++ b/pkg/engine/variables/evaluate.go @@ -28,9 +28,9 @@ func EvaluateConditions(log logr.Logger, ctx context.EvalInterface, conditions i return false } -func EvaluateAnyAllConditions(log logr.Logger, ctx context.EvalInterface, conditions []*kyverno.AnyAllConditions) bool { +func EvaluateAnyAllConditions(log logr.Logger, ctx context.EvalInterface, conditions []kyverno.AnyAllConditions) bool { for _, c := range conditions { - if !evaluateAnyAllConditions(log, ctx, *c) { + if !evaluateAnyAllConditions(log, ctx, c) { return false } } diff --git a/pkg/engine/variables/vars.go b/pkg/engine/variables/vars.go index 73df1126f2..892767f2a2 100644 --- a/pkg/engine/variables/vars.go +++ b/pkg/engine/variables/vars.go @@ -138,7 +138,7 @@ func UntypedToRule(untyped interface{}) (kyverno.Rule, error) { return rule, nil } -func SubstituteAllInConditions(log logr.Logger, ctx context.EvalInterface, conditions []*kyverno.AnyAllConditions) ([]*kyverno.AnyAllConditions, error) { +func SubstituteAllInConditions(log logr.Logger, ctx context.EvalInterface, conditions []kyverno.AnyAllConditions) ([]kyverno.AnyAllConditions, error) { c, err := ConditionsToJSONObject(conditions) if err != nil { return nil, err @@ -152,7 +152,7 @@ func SubstituteAllInConditions(log logr.Logger, ctx context.EvalInterface, condi return JSONObjectToConditions(i) } -func ConditionsToJSONObject(conditions []*kyverno.AnyAllConditions) ([]map[string]interface{}, error) { +func ConditionsToJSONObject(conditions []kyverno.AnyAllConditions) ([]map[string]interface{}, error) { bytes, err := json.Marshal(conditions) if err != nil { return nil, err @@ -166,13 +166,13 @@ func ConditionsToJSONObject(conditions []*kyverno.AnyAllConditions) ([]map[strin return m, nil } -func JSONObjectToConditions(data interface{}) ([]*kyverno.AnyAllConditions, error) { +func JSONObjectToConditions(data interface{}) ([]kyverno.AnyAllConditions, error) { bytes, err := json.Marshal(data) if err != nil { return nil, err } - var c []*kyverno.AnyAllConditions + var c []kyverno.AnyAllConditions if err := json.Unmarshal(bytes, &c); err != nil { return nil, err }