Skip to content

Commit 59c2b8c

Browse files
authored
feat: upload without tags and sign only manifest (#32)
* feat: upload without tags and sign only manifest * feat: add architecture and os into the config of the binary for multi arch * feat: push artifacts without remote tag * feat: push artifacts without remote tag
1 parent 750d6f7 commit 59c2b8c

File tree

11 files changed

+209
-285
lines changed

11 files changed

+209
-285
lines changed

cmd/agent-metadata-action/main.go

Lines changed: 24 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ var createMetadataClientFunc = func(baseURL, token string) metadataClient {
3333

3434
// ociHandleUploadsFunc is a variable that holds the function to handle OCI uploads
3535
// This allows tests to override the implementation
36-
var ociHandleUploadsFunc = oci.HandleUploads
36+
var ociHandleUploadsFunc = func(ctx context.Context, ociConfig *models.OCIConfig, workspace, version string) (string, error) {
37+
return oci.HandleUploads(ctx, ociConfig, workspace, version)
38+
}
3739

3840
// initNewRelic initializes the New Relic application
3941
// Returns nil if APM_CONTROL_NR_LICENSE_KEY is not set (silent no-op mode)
@@ -238,41 +240,30 @@ func runAgentFlow(ctx context.Context, client metadataClient, workspace, agentTy
238240
return fmt.Errorf("error loading OCI config: %w", err)
239241
}
240242

241-
// Step 2: Upload binaries
242-
uploadResults, err := ociHandleUploadsFunc(ctx, &ociConfig, workspace, agentType, agentVersion)
243-
if err != nil {
244-
return fmt.Errorf("binary upload failed: %w", err)
245-
}
246-
247-
// Step 3: Sign all uploaded artifacts
248243
if ociConfig.IsEnabled() {
249-
successfulUploads := []models.ArtifactUploadResult{}
250-
for _, result := range uploadResults {
251-
if result.Uploaded {
252-
successfulUploads = append(successfulUploads, result)
253-
}
244+
// Step 2: Upload binaries
245+
indexDigest, err := ociHandleUploadsFunc(ctx, &ociConfig, workspace, agentVersion)
246+
if err != nil {
247+
return fmt.Errorf("binary upload failed: %w", err)
248+
}
249+
250+
// Step 3: Sign the manifest index
251+
githubRepo := config.GetRepo()
252+
if githubRepo == "" {
253+
return fmt.Errorf("GITHUB_REPOSITORY environment variable is required for artifact signing")
254+
}
255+
256+
// Extract repository name from full path (e.g., "agent-metadata-action" from "newrelic/agent-metadata-action")
257+
repoParts := strings.Split(githubRepo, "/")
258+
repoName := repoParts[len(repoParts)-1]
259+
260+
token := config.GetToken()
261+
if token == "" {
262+
return fmt.Errorf("NEWRELIC_TOKEN is required for artifact signing")
254263
}
255264

256-
if len(successfulUploads) > 0 {
257-
githubRepo := config.GetRepo()
258-
if githubRepo == "" {
259-
return fmt.Errorf("GITHUB_REPOSITORY environment variable is required for artifact signing")
260-
}
261-
262-
// Extract repository name from full path (e.g., "agent-metadata-action" from "newrelic/agent-metadata-action")
263-
repoParts := strings.Split(githubRepo, "/")
264-
repoName := repoParts[len(repoParts)-1]
265-
266-
token := config.GetToken()
267-
if token == "" {
268-
return fmt.Errorf("NEWRELIC_TOKEN is required for artifact signing")
269-
}
270-
271-
if err := sign.SignArtifacts(ctx, successfulUploads, ociConfig.Registry, token, repoName, agentVersion); err != nil {
272-
return fmt.Errorf("artifact signing failed: %w", err)
273-
}
274-
} else {
275-
logging.Warn(ctx, "OCI registry is enabled but no artifacts were successfully uploaded")
265+
if err := sign.SignIndex(ctx, ociConfig.Registry, indexDigest, agentVersion, token, repoName); err != nil {
266+
return fmt.Errorf("artifact signing failed: %w", err)
276267
}
277268
}
278269

cmd/agent-metadata-action/main_test.go

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package main
33
import (
44
"context"
55
"encoding/json"
6+
"fmt"
67
"io"
78
"net/http"
89
"net/http/httptest"
@@ -585,12 +586,10 @@ func TestRunAgentFlow_SigningSuccess_SingleArtifact(t *testing.T) {
585586
}
586587
defer func() { createMetadataClientFunc = originalCreateClient }()
587588

588-
// Mock OCI handler to return 1 successful upload
589+
// Mock OCI handler to return index digest
589590
originalOCIHandler := ociHandleUploadsFunc
590-
ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
591-
return []models.ArtifactUploadResult{
592-
createSuccessfulUploadResult("linux-tar", "sha256:abc123", "v1.2.3-linux-amd64"),
593-
}, nil
591+
ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, version string) (string, error) {
592+
return "sha256:index123", nil
594593
}
595594
defer func() { ociHandleUploadsFunc = originalOCIHandler }()
596595

@@ -604,14 +603,14 @@ func TestRunAgentFlow_SigningSuccess_SingleArtifact(t *testing.T) {
604603
assert.Equal(t, "/v1/signing/agent-metadata-action/sign", r.URL.Path)
605604
assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization"))
606605

607-
// Validate request body
606+
// Validate request body - should be signing the manifest index
608607
body, _ := io.ReadAll(r.Body)
609608
var signingReq models.SigningRequest
610609
json.Unmarshal(body, &signingReq)
611610
assert.Equal(t, "docker.io", signingReq.Registry)
612611
assert.Equal(t, "newrelic/agents", signingReq.Repository)
613-
assert.Equal(t, "v1.2.3-linux-amd64", signingReq.Tag)
614-
assert.Equal(t, "sha256:abc123", signingReq.Digest)
612+
assert.Equal(t, "1.2.3", signingReq.Tag)
613+
assert.Equal(t, "sha256:index123", signingReq.Digest)
615614

616615
w.WriteHeader(http.StatusOK)
617616
w.Write([]byte(`{"success": true}`))
@@ -649,10 +648,9 @@ func TestRunAgentFlow_SigningSuccess_SingleArtifact(t *testing.T) {
649648
// Verify signing requests
650649
assert.Equal(t, 1, requestCount, "Should have made 1 signing request")
651650

652-
// Verify logging
653-
assert.Contains(t, outputStr, "Starting artifact signing for 1 artifacts")
654-
assert.Contains(t, outputStr, "Successfully signed artifact linux-tar")
655-
assert.Contains(t, outputStr, "Artifact signing complete: 1/1 signed successfully")
651+
// Verify logging - now signing the manifest index
652+
assert.Contains(t, outputStr, "Starting manifest index signing")
653+
assert.Contains(t, outputStr, "Successfully signed manifest index")
656654
assert.NotContains(t, stderrStr, "::error::")
657655
}
658656

@@ -666,8 +664,9 @@ func TestRunAgentFlow_SigningDisabled_OCINotEnabled(t *testing.T) {
666664

667665
// Mock OCI handler should not be called since OCI is disabled
668666
originalOCIHandler := ociHandleUploadsFunc
669-
ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
670-
return []models.ArtifactUploadResult{}, nil
667+
ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, version string) (string, error) {
668+
t.Fatal("OCI handler should not be called when OCI is disabled")
669+
return "", nil
671670
}
672671
defer func() { ociHandleUploadsFunc = originalOCIHandler }()
673672

@@ -706,7 +705,7 @@ func TestRunAgentFlow_SigningDisabled_OCINotEnabled(t *testing.T) {
706705

707706
// Verify NO signing requests were made
708707
assert.Equal(t, 0, requestCount, "Should have made 0 signing requests")
709-
assert.NotContains(t, outputStr, "Starting artifact signing")
708+
assert.NotContains(t, outputStr, "Starting manifest index signing")
710709
assert.NotContains(t, stderrStr, "::error::")
711710
}
712711

@@ -718,13 +717,10 @@ func TestRunAgentFlow_SigningSkipped_AllUploadsFailed(t *testing.T) {
718717
}
719718
defer func() { createMetadataClientFunc = originalCreateClient }()
720719

721-
// Mock OCI handler to return only failed uploads
720+
// Mock OCI handler to return error (fail-fast behavior)
722721
originalOCIHandler := ociHandleUploadsFunc
723-
ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
724-
return []models.ArtifactUploadResult{
725-
createFailedUploadResult("linux-tar"),
726-
createFailedUploadResult("windows-zip"),
727-
}, nil
722+
ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, version string) (string, error) {
723+
return "", fmt.Errorf("artifact upload failed for linux-tar: upload error")
728724
}
729725
defer func() { ociHandleUploadsFunc = originalOCIHandler }()
730726

@@ -756,17 +752,15 @@ func TestRunAgentFlow_SigningSkipped_AllUploadsFailed(t *testing.T) {
756752
mockClient := &mockMetadataClient{}
757753
err = runAgentFlow(ctx, mockClient, workspace, "java", "1.2.3")
758754

759-
// Verify success (signing is skipped but flow continues)
760-
require.NoError(t, err)
755+
// Verify error (fail-fast behavior)
756+
require.Error(t, err)
757+
assert.Contains(t, err.Error(), "binary upload failed")
761758

762759
outputStr := getStdout()
763760

764761
// Verify NO signing requests were made
765762
assert.Equal(t, 0, requestCount, "Should have made 0 signing requests")
766-
767-
// Verify warning was logged
768-
assert.Contains(t, outputStr, "OCI registry is enabled but no artifacts were successfully uploaded")
769-
assert.NotContains(t, outputStr, "Starting artifact signing")
763+
assert.NotContains(t, outputStr, "Starting manifest index signing")
770764
}
771765

772766
func TestRunAgentFlow_SigningError_ServiceFailure(t *testing.T) {
@@ -777,12 +771,10 @@ func TestRunAgentFlow_SigningError_ServiceFailure(t *testing.T) {
777771
}
778772
defer func() { createMetadataClientFunc = originalCreateClient }()
779773

780-
// Mock OCI handler to return 1 successful upload
774+
// Mock OCI handler to return index digest
781775
originalOCIHandler := ociHandleUploadsFunc
782-
ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, agentType, version string) ([]models.ArtifactUploadResult, error) {
783-
return []models.ArtifactUploadResult{
784-
createSuccessfulUploadResult("linux-tar", "sha256:abc123", "v1.2.3-linux-amd64"),
785-
}, nil
776+
ociHandleUploadsFunc = func(ctx context.Context, cfg *models.OCIConfig, workspace, version string) (string, error) {
777+
return "sha256:index123", nil
786778
}
787779
defer func() { ociHandleUploadsFunc = originalOCIHandler }()
788780

@@ -825,5 +817,5 @@ func TestRunAgentFlow_SigningError_ServiceFailure(t *testing.T) {
825817
assert.Equal(t, 3, requestCount, "Should have made 3 signing requests (retries)")
826818
assert.Contains(t, outputStr, "Signing attempt 1 failed")
827819
assert.Contains(t, outputStr, "Signing attempt 2 failed")
828-
assert.Contains(t, outputStr, "Failed to sign artifact linux-tar after 3 attempts")
820+
assert.Contains(t, outputStr, "Failed to sign manifest index after 3 attempts")
829821
}

internal/models/binary.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,11 @@ func (a *ArtifactDefinition) Validate() error {
6363
}
6464

6565
func (a *ArtifactDefinition) GetMediaType() string {
66-
return fmt.Sprintf("application/vnd.newrelic.agent.v1+%s", a.Format)
66+
return fmt.Sprintf("application/vnd.newrelic.agent.content.v1.%s", a.Format)
6767
}
6868

6969
func (a *ArtifactDefinition) GetArtifactType() string {
70-
return fmt.Sprintf("application/vnd.newrelic.agent.v1+%s", a.Format)
70+
return "application/vnd.newrelic.agent.v1"
7171
}
7272

7373
func (a *ArtifactDefinition) GetPlatformString() string {

internal/models/binary_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,9 @@ func TestArtifactDefinition_GetMediaType(t *testing.T) {
190190
format string
191191
expected string
192192
}{
193-
{"tar", "application/vnd.newrelic.agent.v1+tar"},
194-
{"tar+gzip", "application/vnd.newrelic.agent.v1+tar+gzip"},
195-
{"zip", "application/vnd.newrelic.agent.v1+zip"},
193+
{"tar", "application/vnd.newrelic.agent.content.v1.tar"},
194+
{"tar+gzip", "application/vnd.newrelic.agent.content.v1.tar+gzip"},
195+
{"zip", "application/vnd.newrelic.agent.content.v1.zip"},
196196
}
197197

198198
for _, tt := range tests {
@@ -205,7 +205,7 @@ func TestArtifactDefinition_GetMediaType(t *testing.T) {
205205

206206
func TestArtifactDefinition_GetArtifactType(t *testing.T) {
207207
artifact := ArtifactDefinition{Format: "tar+gzip"}
208-
assert.Equal(t, "application/vnd.newrelic.agent.v1+tar+gzip", artifact.GetArtifactType())
208+
assert.Equal(t, "application/vnd.newrelic.agent.v1", artifact.GetArtifactType())
209209
}
210210

211211
func TestArtifactDefinition_GetPlatformString(t *testing.T) {

internal/oci/client.go

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -55,71 +55,82 @@ func NewClient(ctx context.Context, registry, username, password string) (*Clien
5555
}, nil
5656
}
5757

58-
func (c *Client) UploadArtifact(ctx context.Context, artifact *models.ArtifactDefinition, artifactPath, agentType, version string) (string, int64, string, error) {
58+
func (c *Client) UploadArtifact(ctx context.Context, artifact *models.ArtifactDefinition, artifactPath, version string) (string, int64, error) {
5959
tempDir, err := os.MkdirTemp("", "oras-upload-*")
6060
if err != nil {
61-
return "", 0, "", fmt.Errorf("failed to create temp directory: %w", err)
61+
return "", 0, fmt.Errorf("failed to create temp directory: %w", err)
6262
}
6363
defer os.RemoveAll(tempDir)
6464

6565
fs, err := file.New(tempDir)
6666
if err != nil {
67-
return "", 0, "", fmt.Errorf("failed to create file store: %w", err)
67+
return "", 0, fmt.Errorf("failed to create file store: %w", err)
6868
}
6969
defer fs.Close()
7070

7171
layerAnnotations := CreateLayerAnnotations(artifact, version)
7272

7373
layerDesc, err := fs.Add(ctx, artifact.Name, artifact.GetMediaType(), artifactPath)
7474
if err != nil {
75-
return "", 0, "", fmt.Errorf("failed to add file to store: %w", err)
75+
return "", 0, fmt.Errorf("failed to add file to store: %w", err)
7676
}
7777

7878
layerDesc.Annotations = layerAnnotations
7979

8080
manifestAnnotations := CreateManifestAnnotations()
8181

82-
emptyConfig := []byte("{}")
83-
emptyConfigDesc := ocispec.Descriptor{
84-
MediaType: "application/vnd.oci.empty.v1+json",
85-
Digest: digest.FromBytes(emptyConfig),
86-
Size: int64(len(emptyConfig)),
82+
// Create config with platform information for multi-arch support
83+
config := map[string]string{
84+
"architecture": artifact.Arch,
85+
"os": artifact.OS,
86+
}
87+
configBytes, err := json.Marshal(config)
88+
if err != nil {
89+
return "", 0, fmt.Errorf("failed to marshal config: %w", err)
90+
}
91+
92+
configDesc := ocispec.Descriptor{
93+
MediaType: "application/vnd.newrelic.agent.config.v1+json",
94+
Digest: digest.FromBytes(configBytes),
95+
Size: int64(len(configBytes)),
8796
}
8897

89-
if err = fs.Push(ctx, emptyConfigDesc, bytes.NewReader(emptyConfig)); err != nil {
90-
return "", 0, "", fmt.Errorf("failed to push empty config: %w", err)
98+
if err = fs.Push(ctx, configDesc, bytes.NewReader(configBytes)); err != nil {
99+
return "", 0, fmt.Errorf("failed to push config: %w", err)
91100
}
92101

93102
artifactType := artifact.GetArtifactType()
94103
packOpts := oras.PackManifestOptions{
95-
ConfigDescriptor: &emptyConfigDesc,
104+
ConfigDescriptor: &configDesc,
96105
Layers: []ocispec.Descriptor{layerDesc},
97106
ManifestAnnotations: manifestAnnotations,
98107
}
99108

100109
manifestDesc, err := oras.PackManifest(ctx, fs, oras.PackManifestVersion1_1, artifactType, packOpts)
101110
if err != nil {
102-
return "", 0, "", fmt.Errorf("failed to pack manifest: %w", err)
111+
return "", 0, fmt.Errorf("failed to pack manifest: %w", err)
103112
}
104113

105-
artifactTag := fmt.Sprintf("%s-%s-%s", version, artifact.OS, artifact.Arch)
106-
107-
// Tag in file store so we can reference it during copy
108-
if err = fs.Tag(ctx, manifestDesc, artifactTag); err != nil {
109-
return "", 0, "", fmt.Errorf("failed to tag manifest: %w", err)
114+
// Tag manifest in file store with a temporary tag so it can be referenced during copy
115+
tempTag := "temp-manifest"
116+
if err = fs.Tag(ctx, manifestDesc, tempTag); err != nil {
117+
return "", 0, fmt.Errorf("failed to tag manifest in file store: %w", err)
110118
}
111119

112-
copyCtx, cancel := context.WithTimeout(ctx, 5*time.Minute)
120+
pushCtx, cancel := context.WithTimeout(ctx, 5*time.Minute)
113121
defer cancel()
114122

123+
logging.Debugf(ctx, "Pushing artifact %s to registry by digest (digest: %s)", artifact.Name, manifestDesc.Digest.String())
124+
125+
// Copy manifest and blobs to remote registry by digest
115126
copyOpts := oras.CopyOptions{}
116-
logging.Debugf(ctx, "Copying artifact %s to registry with tag %s (digest: %s)", artifact.Name, artifactTag, manifestDesc.Digest.String())
117-
if _, err = oras.Copy(copyCtx, fs, artifactTag, c.repo, artifactTag, copyOpts); err != nil {
118-
return "", 0, "", fmt.Errorf("failed to copy artifact to registry: %w", err)
127+
digestRef := manifestDesc.Digest.String()
128+
if _, err = oras.Copy(pushCtx, fs, tempTag, c.repo, digestRef, copyOpts); err != nil {
129+
return "", 0, fmt.Errorf("failed to push artifact to registry: %w", err)
119130
}
120131

121-
logging.Debugf(ctx, "Successfully uploaded artifact with tag %s", artifactTag)
122-
return manifestDesc.Digest.String(), manifestDesc.Size, artifactTag, nil
132+
logging.Debugf(ctx, "Successfully uploaded artifact by digest: %s", manifestDesc.Digest.String())
133+
return manifestDesc.Digest.String(), manifestDesc.Size, nil
123134
}
124135

125136
func (c *Client) CreateManifestIndex(ctx context.Context, uploadResults []models.ArtifactUploadResult, version string) (string, error) {
@@ -141,14 +152,12 @@ func (c *Client) CreateManifestIndex(ctx context.Context, uploadResults []models
141152
Architecture: result.Arch,
142153
}
143154

144-
artifactType := fmt.Sprintf("application/vnd.newrelic.agent.v1+%s", result.Format)
145-
146155
manifest := ocispec.Descriptor{
147156
MediaType: ocispec.MediaTypeImageManifest,
148157
Digest: digest,
149158
Size: result.Size,
150159
Platform: platform,
151-
ArtifactType: artifactType,
160+
ArtifactType: "application/vnd.newrelic.agent.v1",
152161
}
153162

154163
manifests = append(manifests, manifest)

0 commit comments

Comments
 (0)