diff --git a/app/cli/cmd/attestation_add.go b/app/cli/cmd/attestation_add.go index abbbbe905..e6ec1459b 100644 --- a/app/cli/cmd/attestation_add.go +++ b/app/cli/cmd/attestation_add.go @@ -17,6 +17,8 @@ package cmd import ( "errors" + "fmt" + "strings" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -28,6 +30,7 @@ import ( func newAttestationAddCmd() *cobra.Command { var name, value string var artifactCASConn *grpc.ClientConn + var annotationsFlag []string cmd := &cobra.Command{ Use: "add", @@ -44,7 +47,17 @@ func newAttestationAddCmd() *cobra.Command { }, ) - err := a.Run(name, value) + // Extract annotations + var annotations = make(map[string]string) + for _, annotation := range annotationsFlag { + kv := strings.SplitN(annotation, "=", 2) + if len(kv) != 2 { + return fmt.Errorf("invalid annotation %q, the format must be key=value", annotation) + } + annotations[kv[0]] = kv[1] + } + + err := a.Run(name, value, annotations) if err != nil { if errors.Is(err, action.ErrAttestationNotInitialized) { return err @@ -67,11 +80,12 @@ func newAttestationAddCmd() *cobra.Command { } cmd.Flags().StringVar(&name, "name", "", "name of the material to be recorded") - cmd.Flags().StringVar(&value, "value", "", "value to be recorded") err := cmd.MarkFlagRequired("name") cobra.CheckErr(err) + cmd.Flags().StringVar(&value, "value", "", "value to be recorded") err = cmd.MarkFlagRequired("value") cobra.CheckErr(err) + cmd.Flags().StringSliceVar(&annotationsFlag, "annotation", nil, "additional annotation in the format of key=value") return cmd } diff --git a/app/cli/internal/action/attestation_add.go b/app/cli/internal/action/attestation_add.go index c7fe37d30..edd91464a 100644 --- a/app/cli/internal/action/attestation_add.go +++ b/app/cli/internal/action/attestation_add.go @@ -54,7 +54,7 @@ func NewAttestationAdd(cfg *AttestationAddOpts) *AttestationAdd { var ErrAttestationNotInitialized = errors.New("attestation not yet initialized") -func (action *AttestationAdd) Run(k, v string) error { +func (action *AttestationAdd) Run(k, v string, annotations map[string]string) error { if initialized := action.c.AlreadyInitialized(); !initialized { return ErrAttestationNotInitialized } @@ -98,7 +98,7 @@ func (action *AttestationAdd) Run(k, v string) error { casBackend.Uploader = casclient.New(artifactCASConn, casclient.WithLogger(action.Logger)) } - if err := action.c.AddMaterial(k, v, casBackend); err != nil { + if err := action.c.AddMaterial(k, v, casBackend, annotations); err != nil { return fmt.Errorf("adding material: %w", err) } diff --git a/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts b/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts index 9966b66a0..85414c910 100644 --- a/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts +++ b/app/controlplane/api/gen/frontend/workflowcontract/v1/crafting_schema.ts @@ -139,12 +139,7 @@ export function craftingSchema_Material_MaterialTypeToJSON(object: CraftingSchem export interface Annotation { /** Single word optionally separated with _ */ name: string; - /** - * TODO: This value is required for now, but this behavior will chance once we allow - * The user to set the value during attestation - * The value can be set in the contract or during the attestation process - * that's why its value is optional - */ + /** This value can be set in the contract or provided during the attestation */ value: string; } diff --git a/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go b/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go index 09b5e3000..5d1688041 100644 --- a/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go +++ b/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.go @@ -226,10 +226,7 @@ type Annotation struct { unknownFields protoimpl.UnknownFields Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` // Single word optionally separated with _ - // TODO: This value is required for now, but this behavior will chance once we allow - // The user to set the value during attestation - // The value can be set in the contract or during the attestation process - // that's why its value is optional + // This value can be set in the contract or provided during the attestation Value string `protobuf:"bytes,2,opt,name=value,proto3" json:"value,omitempty"` } @@ -469,17 +466,17 @@ var file_workflowcontract_v1_crafting_schema_proto_rawDesc = []byte{ 0x4d, 0x5f, 0x43, 0x59, 0x43, 0x4c, 0x4f, 0x4e, 0x45, 0x44, 0x58, 0x5f, 0x4a, 0x53, 0x4f, 0x4e, 0x10, 0x04, 0x12, 0x12, 0x0a, 0x0e, 0x53, 0x42, 0x4f, 0x4d, 0x5f, 0x53, 0x50, 0x44, 0x58, 0x5f, 0x4a, 0x53, 0x4f, 0x4e, 0x10, 0x05, 0x12, 0x0d, 0x0a, 0x09, 0x4a, 0x55, 0x4e, 0x49, 0x54, 0x5f, - 0x58, 0x4d, 0x4c, 0x10, 0x06, 0x22, 0x4f, 0x0a, 0x0a, 0x41, 0x6e, 0x6e, 0x6f, 0x74, 0x61, 0x74, + 0x58, 0x4d, 0x4c, 0x10, 0x06, 0x22, 0x46, 0x0a, 0x0a, 0x41, 0x6e, 0x6e, 0x6f, 0x74, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x12, 0x22, 0x0a, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x42, 0x0e, 0xfa, 0x42, 0x0b, 0x72, 0x09, 0x32, 0x07, 0x5e, 0x5b, 0x5c, 0x77, 0x5d, 0x2b, - 0x24, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x1d, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, - 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x42, 0x07, 0xfa, 0x42, 0x04, 0x72, 0x02, 0x10, 0x01, 0x52, - 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x42, 0x4d, 0x5a, 0x4b, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, - 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x6c, 0x6f, 0x6f, 0x70, 0x2d, 0x64, - 0x65, 0x76, 0x2f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x6c, 0x6f, 0x6f, 0x70, 0x2f, 0x61, 0x70, 0x70, - 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x70, 0x6c, 0x61, 0x6e, 0x65, 0x2f, 0x61, 0x70, - 0x69, 0x2f, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, - 0x63, 0x74, 0x2f, 0x76, 0x31, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, + 0x24, 0x52, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, + 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x42, 0x4d, 0x5a, + 0x4b, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x63, 0x68, 0x61, 0x69, + 0x6e, 0x6c, 0x6f, 0x6f, 0x70, 0x2d, 0x64, 0x65, 0x76, 0x2f, 0x63, 0x68, 0x61, 0x69, 0x6e, 0x6c, + 0x6f, 0x6f, 0x70, 0x2f, 0x61, 0x70, 0x70, 0x2f, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x6f, 0x6c, 0x70, + 0x6c, 0x61, 0x6e, 0x65, 0x2f, 0x61, 0x70, 0x69, 0x2f, 0x77, 0x6f, 0x72, 0x6b, 0x66, 0x6c, 0x6f, + 0x77, 0x63, 0x6f, 0x6e, 0x74, 0x72, 0x61, 0x63, 0x74, 0x2f, 0x76, 0x31, 0x62, 0x06, 0x70, 0x72, + 0x6f, 0x74, 0x6f, 0x33, } var ( diff --git a/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.validate.go b/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.validate.go index 3300fadf4..26ea19829 100644 --- a/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.validate.go +++ b/app/controlplane/api/workflowcontract/v1/crafting_schema.pb.validate.go @@ -242,16 +242,7 @@ func (m *Annotation) validate(all bool) error { errors = append(errors, err) } - if utf8.RuneCountInString(m.GetValue()) < 1 { - err := AnnotationValidationError{ - field: "Value", - reason: "value length must be at least 1 runes", - } - if !all { - return err - } - errors = append(errors, err) - } + // no validation rules for Value if len(errors) > 0 { return AnnotationMultiError(errors) diff --git a/app/controlplane/api/workflowcontract/v1/crafting_schema.proto b/app/controlplane/api/workflowcontract/v1/crafting_schema.proto index 372627f06..aea2baeb3 100644 --- a/app/controlplane/api/workflowcontract/v1/crafting_schema.proto +++ b/app/controlplane/api/workflowcontract/v1/crafting_schema.proto @@ -65,7 +65,6 @@ message CraftingSchema { message Annotation { string name = 1 [(validate.rules).string.pattern = "^[\\w]+$"]; // Single word optionally separated with _ - // TODO: This value is required for now, but this behavior will change once we allow - // the user to set the value during attestation - string value = 2 [(validate.rules).string.min_len = 1]; + // This value can be set in the contract or provided during the attestation + string value = 2; } \ No newline at end of file diff --git a/app/controlplane/api/workflowcontract/v1/crafting_schema_test.go b/app/controlplane/api/workflowcontract/v1/crafting_schema_test.go index 75bcd0447..d634f4fa4 100644 --- a/app/controlplane/api/workflowcontract/v1/crafting_schema_test.go +++ b/app/controlplane/api/workflowcontract/v1/crafting_schema_test.go @@ -41,11 +41,6 @@ func TestValidateAnnotations(t *testing.T) { value: "hi", wantErr: true, }, - { - desc: "missing value", - name: "hi", - wantErr: true, - }, { desc: "valid key underscore", name: "hello_world", diff --git a/internal/attestation/crafter/crafter.go b/internal/attestation/crafter/crafter.go index 01ad6a0fd..d879096bb 100644 --- a/internal/attestation/crafter/crafter.go +++ b/internal/attestation/crafter/crafter.go @@ -326,7 +326,7 @@ func notResolvedVars(resolved map[string]string, wantList []string) []string { } // Inject material to attestation state -func (c *Crafter) AddMaterial(key, value string, casBackend *casclient.CASBackend) error { +func (c *Crafter) AddMaterial(key, value string, casBackend *casclient.CASBackend, runtimeAnnotations map[string]string) error { if err := c.requireStateLoaded(); err != nil { return err } @@ -354,11 +354,40 @@ func (c *Crafter) AddMaterial(key, value string, casBackend *casclient.CASBacken return err } + // 4 - Populate annotations from the ones provided at runtime + // a) we do not allow overriding values that come from the contract + // b) we do not allow adding annotations that are not defined in the contract + for kr, vr := range runtimeAnnotations { + // If the annotation is not defined in the material we fail + if v, found := mt.Annotations[kr]; !found { + return fmt.Errorf("annotation %q not found in material %q", kr, key) + } else if v == "" { + // Set it only if it's not set + mt.Annotations[kr] = vr + } else { + // NOTE: we do not allow overriding values that come from the contract + c.logger.Info().Str("key", key).Str("annotation", kr).Msg("annotation can't be changed, skipping") + } + } + + // Make sure all the annotation values are now set + // This is in fact validated below but by manually checking we can provide a better error message + for k, v := range mt.Annotations { + var missingAnnotations []string + if v == "" { + missingAnnotations = append(missingAnnotations, k) + } + + if len(missingAnnotations) > 0 { + return fmt.Errorf("annotations %q required for material %q", missingAnnotations, key) + } + } + if err := mt.Validate(); err != nil { return fmt.Errorf("validation error: %w", err) } - // 4 - Attach it to state + // 5 - Attach it to state if mt != nil { if c.CraftingState.Attestation.Materials == nil { c.CraftingState.Attestation.Materials = map[string]*api.Attestation_Material{key: mt} @@ -366,7 +395,7 @@ func (c *Crafter) AddMaterial(key, value string, casBackend *casclient.CASBacken c.CraftingState.Attestation.Materials[key] = mt } - // 5 - Persist state + // 6 - Persist state if err := persistCraftingState(c.CraftingState, c.statePath); err != nil { return err }