diff --git a/CLAUDE.md b/CLAUDE.md index c2a6f82ff..0f6d148eb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -259,6 +259,8 @@ Code reviews are required for all submissions via GitHub pull requests. - I do not want you to be in the co-author signoff - when the schema is changed, run make generate, do not create a migration explicitly - If you are writing go code, adhere to best practices such as the ones in effective-go, or others. This could include, error handling patterns, interface design, package organization, concurrency patterns, etc. +- When writing tests, use table-driven tests whenever possible +- When implementing new functionality, follow TDD: write failing tests first, then implement the code to make them pass - do not change previous migrations, they are immutable - if you add any new dependency to a constructor, remember to run wire ./... - when adding new inedexes, make sure to update the generated sql migraiton files and make them CREATE INDEX CONCURRENTLY and set -- atlas:txmode none at the top @@ -272,3 +274,4 @@ Code reviews are required for all submissions via GitHub pull requests. - any call to authorization Enforce done from the biz or svc layer must be done using biz.AuthzUseCase - if you modify a schema, remember to run `make migration_sync` - after changing Helm chart source code (`deployment/chainloop/`), bump the **patch** version (not minor, not major) in the chart's `Chart.yaml` +- when asked to create a GitHub issue, create it in the `chainloop-dev/chainloop` repository diff --git a/pkg/attestation/crafter/collector_prmetadata.go b/pkg/attestation/crafter/collector_prmetadata.go index f683a2116..2d75cc8d5 100644 --- a/pkg/attestation/crafter/collector_prmetadata.go +++ b/pkg/attestation/crafter/collector_prmetadata.go @@ -68,20 +68,13 @@ func (c *PRMetadataCollector) Collect(ctx context.Context, cr *Crafter, attestat return fmt.Errorf("marshaling PR/MR metadata: %w", err) } - materialName := fmt.Sprintf("pr-metadata-%s", metadata.Number) - tmpFile, err := os.CreateTemp("", fmt.Sprintf("%s-*.json", materialName)) + materialName, tmpFile, err := createPRMetadataTempFile(metadata.Number, jsonData) if err != nil { - return fmt.Errorf("creating temp file: %w", err) - } - defer os.Remove(tmpFile.Name()) - - if _, err := tmpFile.Write(jsonData); err != nil { - tmpFile.Close() - return fmt.Errorf("writing temp file: %w", err) + return fmt.Errorf("creating PR metadata temp file: %w", err) } - tmpFile.Close() + defer os.Remove(tmpFile) - if _, err := cr.AddMaterialContractFree(ctx, attestationID, schemaapi.CraftingSchema_Material_CHAINLOOP_PR_INFO.String(), materialName, tmpFile.Name(), casBackend, nil); err != nil { + if _, err := cr.AddMaterialContractFree(ctx, attestationID, schemaapi.CraftingSchema_Material_CHAINLOOP_PR_INFO.String(), materialName, tmpFile, casBackend, nil); err != nil { return fmt.Errorf("adding PR/MR metadata material: %w", err) } @@ -89,3 +82,26 @@ func (c *PRMetadataCollector) Collect(ctx context.Context, cr *Crafter, attestat return nil } + +// createPRMetadataTempFile creates a temp file with the PR metadata JSON content +// and returns the material name and the temp file path. +func createPRMetadataTempFile(prNumber string, data []byte) (materialName string, filePath string, err error) { + materialName = fmt.Sprintf("pr-metadata-%s", prNumber) + + tmpFile, err := os.CreateTemp("", fmt.Sprintf("%s-*.json", materialName)) + if err != nil { + return "", "", fmt.Errorf("creating temp file: %w", err) + } + + if _, err := tmpFile.Write(data); err != nil { + tmpFile.Close() + os.Remove(tmpFile.Name()) + return "", "", fmt.Errorf("writing temp file: %w", err) + } + if err := tmpFile.Close(); err != nil { + os.Remove(tmpFile.Name()) + return "", "", fmt.Errorf("closing temp file: %w", err) + } + + return materialName, tmpFile.Name(), nil +} diff --git a/pkg/attestation/crafter/collector_prmetadata_test.go b/pkg/attestation/crafter/collector_prmetadata_test.go new file mode 100644 index 000000000..0ed4ca310 --- /dev/null +++ b/pkg/attestation/crafter/collector_prmetadata_test.go @@ -0,0 +1,66 @@ +// +// Copyright 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. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package crafter + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCreatePRMetadataTempFile(t *testing.T) { + tests := []struct { + name string + prNumber string + wantMaterial string + wantFilePrefix string + }{ + { + name: "numeric PR number", + prNumber: "123", + wantMaterial: "pr-metadata-123", + wantFilePrefix: "pr-metadata-123-", + }, + { + name: "large PR number", + prNumber: "99999", + wantMaterial: "pr-metadata-99999", + wantFilePrefix: "pr-metadata-99999-", + }, + { + name: "single digit PR number", + prNumber: "1", + wantMaterial: "pr-metadata-1", + wantFilePrefix: "pr-metadata-1-", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + materialName, filePath, err := createPRMetadataTempFile(tc.prNumber, []byte(`{"test": true}`)) + require.NoError(t, err) + defer os.Remove(filePath) + + assert.Equal(t, tc.wantMaterial, materialName) + assert.Equal(t, ".json", filepath.Ext(filePath)) + assert.True(t, strings.HasPrefix(filepath.Base(filePath), tc.wantFilePrefix)) + }) + } +} diff --git a/pkg/attestation/crafter/prmetadata_test.go b/pkg/attestation/crafter/prmetadata_test.go index 472234b2f..1e6636839 100644 --- a/pkg/attestation/crafter/prmetadata_test.go +++ b/pkg/attestation/crafter/prmetadata_test.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.