Skip to content

Commit 3cfc998

Browse files
authored
fix: PR metadata temp file missing .json extension (#2846)
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
1 parent 509eafe commit 3cfc998

4 files changed

Lines changed: 97 additions & 12 deletions

File tree

CLAUDE.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,8 @@ Code reviews are required for all submissions via GitHub pull requests.
259259
- I do not want you to be in the co-author signoff
260260
- when the schema is changed, run make generate, do not create a migration explicitly
261261
- 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.
262+
- When writing tests, use table-driven tests whenever possible
263+
- When implementing new functionality, follow TDD: write failing tests first, then implement the code to make them pass
262264
- do not change previous migrations, they are immutable
263265
- if you add any new dependency to a constructor, remember to run wire ./...
264266
- 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.
272274
- any call to authorization Enforce done from the biz or svc layer must be done using biz.AuthzUseCase
273275
- if you modify a schema, remember to run `make migration_sync`
274276
- after changing Helm chart source code (`deployment/chainloop/`), bump the **patch** version (not minor, not major) in the chart's `Chart.yaml`
277+
- when asked to create a GitHub issue, create it in the `chainloop-dev/chainloop` repository

pkg/attestation/crafter/collector_prmetadata.go

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,24 +68,40 @@ func (c *PRMetadataCollector) Collect(ctx context.Context, cr *Crafter, attestat
6868
return fmt.Errorf("marshaling PR/MR metadata: %w", err)
6969
}
7070

71-
materialName := fmt.Sprintf("pr-metadata-%s", metadata.Number)
72-
tmpFile, err := os.CreateTemp("", fmt.Sprintf("%s-*.json", materialName))
71+
materialName, tmpFile, err := createPRMetadataTempFile(metadata.Number, jsonData)
7372
if err != nil {
74-
return fmt.Errorf("creating temp file: %w", err)
75-
}
76-
defer os.Remove(tmpFile.Name())
77-
78-
if _, err := tmpFile.Write(jsonData); err != nil {
79-
tmpFile.Close()
80-
return fmt.Errorf("writing temp file: %w", err)
73+
return fmt.Errorf("creating PR metadata temp file: %w", err)
8174
}
82-
tmpFile.Close()
75+
defer os.Remove(tmpFile)
8376

84-
if _, err := cr.AddMaterialContractFree(ctx, attestationID, schemaapi.CraftingSchema_Material_CHAINLOOP_PR_INFO.String(), materialName, tmpFile.Name(), casBackend, nil); err != nil {
77+
if _, err := cr.AddMaterialContractFree(ctx, attestationID, schemaapi.CraftingSchema_Material_CHAINLOOP_PR_INFO.String(), materialName, tmpFile, casBackend, nil); err != nil {
8578
return fmt.Errorf("adding PR/MR metadata material: %w", err)
8679
}
8780

8881
cr.Logger.Info().Msg("successfully collected and attested PR/MR metadata")
8982

9083
return nil
9184
}
85+
86+
// createPRMetadataTempFile creates a temp file with the PR metadata JSON content
87+
// and returns the material name and the temp file path.
88+
func createPRMetadataTempFile(prNumber string, data []byte) (materialName string, filePath string, err error) {
89+
materialName = fmt.Sprintf("pr-metadata-%s", prNumber)
90+
91+
tmpFile, err := os.CreateTemp("", fmt.Sprintf("%s-*.json", materialName))
92+
if err != nil {
93+
return "", "", fmt.Errorf("creating temp file: %w", err)
94+
}
95+
96+
if _, err := tmpFile.Write(data); err != nil {
97+
tmpFile.Close()
98+
os.Remove(tmpFile.Name())
99+
return "", "", fmt.Errorf("writing temp file: %w", err)
100+
}
101+
if err := tmpFile.Close(); err != nil {
102+
os.Remove(tmpFile.Name())
103+
return "", "", fmt.Errorf("closing temp file: %w", err)
104+
}
105+
106+
return materialName, tmpFile.Name(), nil
107+
}
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
//
2+
// Copyright 2026 The Chainloop Authors.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
package crafter
17+
18+
import (
19+
"os"
20+
"path/filepath"
21+
"strings"
22+
"testing"
23+
24+
"github.com/stretchr/testify/assert"
25+
"github.com/stretchr/testify/require"
26+
)
27+
28+
func TestCreatePRMetadataTempFile(t *testing.T) {
29+
tests := []struct {
30+
name string
31+
prNumber string
32+
wantMaterial string
33+
wantFilePrefix string
34+
}{
35+
{
36+
name: "numeric PR number",
37+
prNumber: "123",
38+
wantMaterial: "pr-metadata-123",
39+
wantFilePrefix: "pr-metadata-123-",
40+
},
41+
{
42+
name: "large PR number",
43+
prNumber: "99999",
44+
wantMaterial: "pr-metadata-99999",
45+
wantFilePrefix: "pr-metadata-99999-",
46+
},
47+
{
48+
name: "single digit PR number",
49+
prNumber: "1",
50+
wantMaterial: "pr-metadata-1",
51+
wantFilePrefix: "pr-metadata-1-",
52+
},
53+
}
54+
55+
for _, tc := range tests {
56+
t.Run(tc.name, func(t *testing.T) {
57+
materialName, filePath, err := createPRMetadataTempFile(tc.prNumber, []byte(`{"test": true}`))
58+
require.NoError(t, err)
59+
defer os.Remove(filePath)
60+
61+
assert.Equal(t, tc.wantMaterial, materialName)
62+
assert.Equal(t, ".json", filepath.Ext(filePath))
63+
assert.True(t, strings.HasPrefix(filepath.Base(filePath), tc.wantFilePrefix))
64+
})
65+
}
66+
}

pkg/attestation/crafter/prmetadata_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
//
2-
// Copyright 2024-2025 The Chainloop Authors.
2+
// Copyright 2024-2026 The Chainloop Authors.
33
//
44
// Licensed under the Apache License, Version 2.0 (the "License");
55
// you may not use this file except in compliance with the License.

0 commit comments

Comments
 (0)