Skip to content

Commit d9e64ff

Browse files
committed
fix(crafter): surface auto-discovery validation errors
Stop probing material kinds when auto-detection hits a protovalidate ValidationError. Schema-level failures, such as invalid material names, are not kind mismatches and should be surfaced directly. Fixes: #2394 Assisted-by: pi Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
1 parent 4429cd2 commit d9e64ff

2 files changed

Lines changed: 23 additions & 7 deletions

File tree

pkg/attestation/crafter/crafter.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -653,31 +653,33 @@ func (c *Crafter) IsMaterialInContract(key string) bool {
653653
// AddMaterialContactFreeWithAutoDetectedKind adds a material to the crafting state checking the incoming material matches any of the
654654
// supported types in validation order. If the material is not found it will return an error.
655655
func (c *Crafter) AddMaterialContactFreeWithAutoDetectedKind(ctx context.Context, attestationID, name, value string, casBackend *casclient.CASBackend, runtimeAnnotations map[string]string, opts ...AddOpt) (*api.Attestation_Material, error) {
656-
var err error
657656
for _, kind := range schemaapi.CraftingMaterialInValidationOrder {
658657
m, err := c.AddMaterialContractFree(ctx, attestationID, kind.String(), name, value, casBackend, runtimeAnnotations, opts...)
659658
if err == nil {
660-
// Successfully added material, return the kind
661659
return m, nil
662660
}
663661

664662
c.Logger.Debug().Err(err).Str("kind", kind.String()).Msg("failed to add material")
665663

666-
// Handle base error for upload and craft errors except the opening file error
667-
// TODO: have an error to detect validation error instead
668664
var policyError *policies.PolicyError
669665
if errors.Is(err, materials.ErrBaseUploadAndCraft) || errors.As(err, &policyError) {
670666
return nil, err
671667
}
672668

673-
// This is a final error, we detected the kind
674669
if v1.IsAttestationStateErrorConflict(err) {
675670
return nil, err
676671
}
672+
673+
// Proto-validation errors (e.g. invalid material name) are schema-level
674+
// failures, not kind mismatches. Stop probing immediately so the real
675+
// error is surfaced to the user instead of being masked by the loop.
676+
var valErr *protovalidate.ValidationError
677+
if errors.As(err, &valErr) {
678+
return nil, err
679+
}
677680
}
678681

679-
// Return an error if no material could be added
680-
return nil, fmt.Errorf("failed to auto-discover material kind: %w", err)
682+
return nil, errors.New("failed to auto-discover material kind")
681683
}
682684

683685
// addMaterials adds the incoming material m to the crafting state

pkg/attestation/crafter/crafter_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -672,6 +672,20 @@ func (s *crafterSuite) TestAddMaterialsAutomatic() {
672672
}
673673
}
674674

675+
func (s *crafterSuite) TestAddMaterialsAutomaticInvalidNameSurfacesValidationError() {
676+
var runner crafter.SupportedRunner = runners.NewGeneric()
677+
uploader := mUploader.NewUploader(s.T())
678+
679+
c, err := newInitializedCrafter(s.T(), "testdata/contracts/empty_generic.yaml", &v1.WorkflowMetadata{}, false, "", runner)
680+
require.NoError(s.T(), err)
681+
682+
m, err := c.AddMaterialContactFreeWithAutoDetectedKind(context.Background(), "random-id", "invalid_name", "./materials/testdata/junit.xml", &casclient.CASBackend{Uploader: uploader}, nil)
683+
require.Error(s.T(), err)
684+
assert.Nil(s.T(), m)
685+
assert.Contains(s.T(), err.Error(), "must contain only lowercase letters, numbers, and hyphens")
686+
assert.NotContains(s.T(), err.Error(), "failed to auto-discover material kind")
687+
}
688+
675689
func loadSchema(path string) (*schemaapi.CraftingSchema, error) {
676690
// Extract json formatted data
677691
content, err := os.ReadFile(filepath.Clean(path))

0 commit comments

Comments
 (0)