From b8a13e39df6e8c87a9c0e32c6f58b58f425cb939 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Insaurralde?= Date: Wed, 27 May 2026 11:55:19 +0200 Subject: [PATCH] perf(protovalidate): cache validator instead of constructing per call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit protovalidate.New() rebuilds the CEL environment, type registry, and message-evaluator cache on every call. Replace per-call constructions with the library's global validator (or protovalidate.GlobalValidator where a Validator handle is required by protoyaml). The library docs mark the Validator as safe for concurrent use; internally it is a sync.Mutex + atomic.Pointer copy-on-write cache, and the library's own benchmarks share one Validator across goroutines. All call sites in this project pass zero options to protovalidate.New(), so the documented safety guarantee applies without caveats. Measured speedups (darwin/arm64, Apple M5, Go 1.26.3): pkg/credentials/manager: ~1700x (1.14 ms -> 672 ns) pkg/attestation/crafter/api/.../state: ~1790x (4.85 ms -> 2.7 us) pkg/attestation/crafter/materials: ~990x (916 us -> 925 ns) Memory per validation drops from 1.5-6 MB to under 2 KB. Assisted-by: Claude Opus 4.7 (1M context) Signed-off-by: Matías Insaurralde Chainloop-Trace-Sessions: 70c6c792-4598-426b-9e75-482bcb10498f, be9960c4-25ac-4bfe-bfff-cf9db58097cb --- app/controlplane/pkg/biz/casclient.go | 7 +----- app/controlplane/pkg/unmarshal/unmarshal.go | 23 +++++++------------ .../v1/crafting_state_validations.go | 9 ++------ pkg/attestation/crafter/crafter.go | 9 +------- .../crafter/materials/materials.go | 7 +----- pkg/credentials/manager/manager.go | 9 ++------ pkg/policies/policies.go | 8 +------ 7 files changed, 16 insertions(+), 56 deletions(-) diff --git a/app/controlplane/pkg/biz/casclient.go b/app/controlplane/pkg/biz/casclient.go index 98f7856de..5e5d9b9f2 100644 --- a/app/controlplane/pkg/biz/casclient.go +++ b/app/controlplane/pkg/biz/casclient.go @@ -167,12 +167,7 @@ func (uc *CASClientUseCase) IsReady(ctx context.Context) (bool, error) { return false, errors.New("missing CAS server configuration") } - v, err := protovalidate.New() - if err != nil { - return false, fmt.Errorf("failed to create validator: %w", err) - } - - if err := v.Validate(uc.casServerConf); err != nil { + if err := protovalidate.Validate(uc.casServerConf); err != nil { return false, fmt.Errorf("invalid CAS client configuration: %w", err) } diff --git a/app/controlplane/pkg/unmarshal/unmarshal.go b/app/controlplane/pkg/unmarshal/unmarshal.go index 258d1a499..d9960975c 100644 --- a/app/controlplane/pkg/unmarshal/unmarshal.go +++ b/app/controlplane/pkg/unmarshal/unmarshal.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -56,17 +56,11 @@ func (v *validatorAdapter) Validate(msg proto.Message) error { return v.validator.Validate(msg) } -func FromRaw(body []byte, format RawFormat, out proto.Message, doValidate bool) error { - var validator protovalidate.Validator - var err error - - if doValidate { - validator, err = protovalidate.New() - if err != nil { - return fmt.Errorf("could not create validator: %w", err) - } - } +// yamlValidator wraps the protovalidate global Validator for use with protoyaml, +// initialised once and reused across calls. +var yamlValidator = &validatorAdapter{validator: protovalidate.GlobalValidator} +func FromRaw(body []byte, format RawFormat, out proto.Message, doValidate bool) error { switch format { case RawFormatJSON: if err := protojson.Unmarshal(body, out); err != nil { @@ -76,7 +70,7 @@ func FromRaw(body []byte, format RawFormat, out proto.Message, doValidate bool) // protoyaml allows validating the contract while unmarshalling yamlOpts := protoyaml.UnmarshalOptions{} if doValidate { - yamlOpts.Validator = &validatorAdapter{validator: validator} + yamlOpts.Validator = yamlValidator } if err := yamlOpts.Unmarshal(body, out); err != nil { @@ -97,9 +91,8 @@ func FromRaw(body []byte, format RawFormat, out proto.Message, doValidate bool) return fmt.Errorf("unsupported format: %s", format) } - if validator != nil { - err = validator.Validate(out) - if err != nil { + if doValidate { + if err := protovalidate.Validate(out); err != nil { return fmt.Errorf("error validating raw message: %w", err) } } diff --git a/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go b/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go index 80ecbc5b8..c7478f56b 100644 --- a/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go +++ b/pkg/attestation/crafter/api/attestation/v1/crafting_state_validations.go @@ -1,5 +1,5 @@ // -// Copyright 2024-2025 The Chainloop Authors. +// Copyright 2024-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -27,15 +27,10 @@ import ( // ValidateComplete makes sure that the crafting state has been completed // before it gets passed to the renderer func (state *CraftingState) ValidateComplete(dryRun bool) error { - validator, err := protovalidate.New() - if err != nil { - return fmt.Errorf("could not create validator: %w", err) - } - // We do not want to validate the schema of the state if we are just doing a dry run // since it's known to not to contain the workflow metadata information if !dryRun { - if err := validator.Validate(state); err != nil { + if err := protovalidate.Validate(state); err != nil { return fmt.Errorf("invalid crafting state: %w", err) } } diff --git a/pkg/attestation/crafter/crafter.go b/pkg/attestation/crafter/crafter.go index 8fb82f2d1..ccf7435e5 100644 --- a/pkg/attestation/crafter/crafter.go +++ b/pkg/attestation/crafter/crafter.go @@ -67,7 +67,6 @@ type Crafter struct { stateManager StateManager // Authn is used to authenticate with the OCI registry ociRegistryAuth authn.Keychain - validator protovalidate.Validator // attestation client is used to load chainloop policies attClient v1.AttestationServiceClient @@ -166,11 +165,6 @@ func (c *Crafter) RunCollectors(ctx context.Context, attestationID string, casBa func NewCrafter(stateManager StateManager, attClient v1.AttestationServiceClient, opts ...NewOpt) (*Crafter, error) { noopLogger := zerolog.Nop() - validator, err := protovalidate.New() - if err != nil { - return nil, fmt.Errorf("creating proto validator: %w", err) - } - cw, _ := os.Getwd() c := &Crafter{ Logger: &noopLogger, @@ -178,7 +172,6 @@ func NewCrafter(stateManager StateManager, attClient v1.AttestationServiceClient stateManager: stateManager, // By default we authenticate with the current user's keychain (i.e ~/.docker/config.json) ociRegistryAuth: authn.DefaultKeychain, - validator: validator, attClient: attClient, } @@ -684,7 +677,7 @@ func (c *Crafter) addMaterial(ctx context.Context, m *schemaapi.CraftingSchema_M } } - if err := c.validator.Validate(mt); err != nil { + if err := protovalidate.Validate(mt); err != nil { return nil, fmt.Errorf("validation error: %w", err) } diff --git a/pkg/attestation/crafter/materials/materials.go b/pkg/attestation/crafter/materials/materials.go index 1bc1ce7ad..fe8c864d7 100644 --- a/pkg/attestation/crafter/materials/materials.go +++ b/pkg/attestation/crafter/materials/materials.go @@ -224,12 +224,7 @@ func Craft(ctx context.Context, materialSchema *schemaapi.CraftingSchema_Materia var crafter Craftable var err error - validator, err := protovalidate.New() - if err != nil { - return nil, fmt.Errorf("could not create validator: %w", err) - } - - if err := validator.Validate(materialSchema); err != nil { + if err := protovalidate.Validate(materialSchema); err != nil { return nil, fmt.Errorf("validating material: %w", err) } diff --git a/pkg/credentials/manager/manager.go b/pkg/credentials/manager/manager.go index 580f19080..c13cc2016 100644 --- a/pkg/credentials/manager/manager.go +++ b/pkg/credentials/manager/manager.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2026 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -56,12 +56,7 @@ func NewFromConfig(conf *api.Credentials, role credentials.Role, l log.Logger) ( } func validateConfig(msg protoreflect.ProtoMessage) error { - validator, err := protovalidate.New() - if err != nil { - return fmt.Errorf("creating validator: %w", err) - } - - return validator.Validate(msg) + return protovalidate.Validate(msg) } func newAzureKBManager(conf *api.Credentials_AzureKeyVault, prefix string, r credentials.Role, l log.Logger) (*azurekv.Manager, error) { diff --git a/pkg/policies/policies.go b/pkg/policies/policies.go index 069ce05a2..602931360 100644 --- a/pkg/policies/policies.go +++ b/pkg/policies/policies.go @@ -705,15 +705,9 @@ func (pv *PolicyVerifier) getLoader(attachment *v1.PolicyAttachment) (Loader, er } func validateResource(m proto.Message) error { - validator, err := protovalidate.New() - if err != nil { + if err := protovalidate.Validate(m); err != nil { return fmt.Errorf("validating policy spec: %w", err) } - err = validator.Validate(m) - if err != nil { - return fmt.Errorf("validating policy spec: %w", err) - } - return nil }