Skip to content

Commit ff4889f

Browse files
authored
feat: add github_release_asset data source (#2514)
* feat: add github_release_asset data source This addresses issue #2513 and adds support for a `github_release_asset` data source. Example of passing acceptance tests: ``` GITHUB_ORGANIZATION=mterwill \ GITHUB_OWNER=mterwill \ TF_ACC=1 \ go test -v ./... -run ^TestAccGithubReleaseAssetDataSource ? github.com/integrations/terraform-provider-github/v6 [no test files] === RUN TestAccGithubReleaseAssetDataSource === RUN TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID === RUN TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID/with_an_anonymous_account provider_utils.go:51: GITHUB_TOKEN environment variable should be empty provider_utils.go:74: Skipping TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID/with_an_anonymous_account which requires anonymous mode === RUN TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID/with_an_individual_account === RUN TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID/with_an_organization_account --- PASS: TestAccGithubReleaseAssetDataSource (11.65s) --- PASS: TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID (11.65s) --- SKIP: TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID/with_an_anonymous_account (0.00s) --- PASS: TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID/with_an_individual_account (8.90s) --- PASS: TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID/with_an_organization_account (2.75s) PASS ok github.com/integrations/terraform-provider-github/v6/github 12.434s ``` Signed-off-by: Mike Ball <[email protected]> * chore(config): test previewHeaderInjectorTransport.RoundTrip Per request of @deiga (#2514 (comment)), this adds tests for previewHeaderInjectorTransport.RoundTrip logic around application/octest-stream header handling. See google/go-github#3392 for context. Signed-off-by: Mike Ball <[email protected]> * chore: github_release_asset data source tests use new test patterns Per code review feedback from @deiga (#2514 (comment)), this updates the github_release_asset data source tests to use the new testing patterns implemented in #2986 This was tested via: ``` $ TF_ACC=1 \ go test -v ./... -run ^TestAccGithubReleaseAssetDataSource ? github.com/integrations/terraform-provider-github/v6 [no test files] === RUN TestAccGithubReleaseAssetDataSource === RUN TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID --- PASS: TestAccGithubReleaseAssetDataSource (5.91s) --- PASS: TestAccGithubReleaseAssetDataSource/queries_specified_asset_ID (5.91s) PASS ok github.com/integrations/terraform-provider-github/v6/github 6.788s ``` Signed-off-by: Mike Ball <[email protected]> * chore(data_source_github_release_asset): use if err := ...; err != nil pattern Per code review feedback (#2514 (comment)) from @stevehipwell, this tweaks the `data_source_github_release_asset` to use the `if err := ...; err != nil` pattern towards the goal of improving readability. Signed-off-by: Mike Ball <[email protected]> * chore(data_source_github_release_asset): improve terseness Use idiomatic `:=` pattern over `var`-pre-defined variables, per code review feedback from @stevehipwell: #2514 (comment) Signed-off-by: Mike Ball <[email protected]> * chore(data_source_github_release_asset_test): inline ComposeTestCheckFunc This inlines `resource.ComposeTestCheckFunc` to improve readability (rather than define it as a variable), per code review feedback from @stevehipwell: #2514 (comment) Signed-off-by: Mike Ball <[email protected]> * chore(data_source_github_release_asset): source config from acc_test.go Per code review feedback from @stevehipwell, this simplifies test configuration: - source test configuration from `testAccConf`, as done elsewhere - remove the ability to override test configuration via `GH_TEST_*` env vars; this is arguably premature optimization and can be added to `acc_test.go` if it's needed in the future Signed-off-by: Mike Ball <[email protected]> * chore(data_source_github_release_asset): remove unnecessary TestCase.Providers Per code review feedback from @deiga, this removes unnecessary Providers configuration from the data_source_github_release_asset tests. #2514 (comment) Signed-off-by: Mike Ball <[email protected]> * fix(data_source_github_release_asset): use composite ID To protect against the possibility that release asset IDs are not globally unique across GitHub, this configures the use of a 3-part composite ID when tracking `data_source_github_release_asset` instances in TF state. This addresses code review feedback from @deiga: #2514 (comment) Signed-off-by: Mike Ball <[email protected]> * fix(data_source_github_release_asset): avoid client mutation Per code review feedback from @stevehipwell (#2514 (comment)), this avoids the use of `client.Repositories.DownloadReleaseAsset` out of concern the function modifies the GitHub client state which could cause unexpected behaviour in Terraform. Signed-off-by: Mike Ball <[email protected]> * fix(data_source_github_release_asset): properly set updated_at This fixes a copy/paste bug; updated_at is now correctly set on the data source. Signed-off-by: Mike Ball <[email protected]> * feat(data_source_github_release_asset): make download configurable Per code review feedback from @stevehipwell (#2514 (review)), this makes the release asset download optional, controlled by a `download_body` attribute, and off by default. Signed-off-by: Mike Ball <[email protected]> * chore(data_source_github_release_asset): use `download_file`/`file` attributes Replace `download_body` argument and `body` attributes w/ `download_file`/`file`, respectively, towards the goal of improving the data source interface. This addresses code review feedback from @stevehipwell: #2514 (comment) Signed-off-by: Mike Ball <[email protected]> * fix(data_source_github_release_asset): base64 encode `file` Per code review feedback from @stevehipwell (#2514 (comment)), this ensures `file` is base64-encoded. Signed-off-by: Mike Ball <[email protected]> * chore(data_source_github_release_asset): adjust file contents attributes Per code review feedback from @stevehipwell, this changes `download_file`/`file` to `download_file_contents`/`file_contents`, respectively: #2514 (comment) Signed-off-by: Mike Ball <[email protected]> * chore(data_source_github_release_asset): improve acc test This improves the `data_source_github_release_asset` acceptance tests to be more TF-focused via use of the `base64decode` function, per code review request of @stevehipwell: #2514 (comment) Signed-off-by: Mike Ball <[email protected]> * chore(data_source_github_release_asset_test): appease linting This commits the result of `gofumpt`, as per lint failures in: https://github.com/integrations/terraform-provider-github/actions/runs/21232219897/job/61621068985?pr=2514 Signed-off-by: Mike Ball <[email protected]> * chore(config_test): appease lint errors This appeases lint warnings in `github/config_test.go`: https://github.com/integrations/terraform-provider-github/actions/runs/21232219897/job/61621068985?pr=2514 Signed-off-by: Mike Ball <[email protected]> * chore(config): address staticcheck errors This addresses the following staticcheck error (seen in https://github.com/integrations/terraform-provider-github/actions/runs/21232219897/job/61621068985?pr=2514): ``` Error: github/config.go:196:13: QF1001: could apply De Morgan's law (staticcheck) ``` Signed-off-by: Mike Ball <[email protected]> * fix(config_test): use canonical HTTP header names in expectedHeaders Go's http.Header canonicalizes header names (e.g., X-GitHub-Api-Version becomes X-Github-Api-Version). The test's unexpected header check was failing because it compared canonical names against non-canonical keys in the expectedHeaders map. Signed-off-by: Mike Ball <[email protected]> --------- Signed-off-by: Mike Ball <[email protected]>
1 parent d1de3e9 commit ff4889f

7 files changed

Lines changed: 585 additions & 6 deletions

File tree

github/acc_test.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ type testAccConfig struct {
4949
testPublicRepository string
5050
testPublicRepositoryOwner string
5151
testPublicReleaseId int
52+
testPublicRelaseAssetId string
53+
testPublicRelaseAssetName string
54+
testPublicReleaseAssetContent string
5255
testPublicTemplateRepository string
5356
testPublicTemplateRepositoryOwner string
5457
testGHActionsAppInstallationId int
@@ -105,11 +108,16 @@ func TestMain(m *testing.M) {
105108
}
106109

107110
config := testAccConfig{
108-
baseURL: baseURL,
109-
authMode: authMode,
110-
testPublicRepository: "terraform-provider-github",
111-
testPublicRepositoryOwner: "integrations",
112-
testPublicReleaseId: 186531906,
111+
baseURL: baseURL,
112+
authMode: authMode,
113+
testPublicRepository: "terraform-provider-github",
114+
testPublicRepositoryOwner: "integrations",
115+
testPublicReleaseId: 186531906,
116+
// The terraform-provider-github_6.4.0_manifest.json asset ID from
117+
// https://github.com/integrations/terraform-provider-github/releases/tag/v6.4.0
118+
testPublicRelaseAssetId: "207956097",
119+
testPublicRelaseAssetName: "terraform-provider-github_6.4.0_manifest.json",
120+
testPublicReleaseAssetContent: "{\n \"version\": 1,\n \"metadata\": {\n \"protocol_versions\": [\n \"5.0\"\n ]\n }\n}",
113121
testPublicTemplateRepository: "template-repository",
114122
testPublicTemplateRepositoryOwner: "template-repository",
115123
testGHActionsAppInstallationId: 15368,

github/config.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,11 @@ func (injector *previewHeaderInjectorTransport) RoundTrip(req *http.Request) (*h
189189
header := req.Header.Get(name)
190190
if header == "" {
191191
header = value
192-
} else {
192+
// NOTE: Some API endpoints expect a single Accept: application/octet-stream header.
193+
// If one has been set, it's necessary to preserve it as-is, without
194+
// appending previewHeaders value.
195+
// See https://github.com/google/go-github/pull/3392
196+
} else if strings.ToLower(name) != "accept" || header != "application/octet-stream" {
193197
header = strings.Join([]string{header, value}, ",")
194198
}
195199
req.Header.Set(name, header)

github/config_test.go

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package github
22

33
import (
44
"context"
5+
"net/http"
6+
"net/http/httptest"
57
"testing"
68

79
"github.com/shurcooL/githubv4"
@@ -300,3 +302,215 @@ func TestAccConfigMeta(t *testing.T) {
300302
}
301303
})
302304
}
305+
306+
func TestPreviewHeaderInjectorTransport_RoundTrip(t *testing.T) {
307+
tests := []struct {
308+
name string
309+
previewHeaders map[string]string
310+
existingHeaders map[string]string
311+
expectedHeaders map[string]string
312+
expectRoundTripCall bool
313+
}{
314+
{
315+
name: "empty preview headers",
316+
previewHeaders: map[string]string{},
317+
existingHeaders: map[string]string{"User-Agent": "test"},
318+
expectedHeaders: map[string]string{"User-Agent": "test"},
319+
expectRoundTripCall: true,
320+
},
321+
{
322+
name: "add new preview header",
323+
previewHeaders: map[string]string{
324+
"Accept": "application/vnd.github.v3+json",
325+
},
326+
existingHeaders: map[string]string{},
327+
expectedHeaders: map[string]string{
328+
"Accept": "application/vnd.github.v3+json",
329+
},
330+
expectRoundTripCall: true,
331+
},
332+
{
333+
name: "append to existing header",
334+
previewHeaders: map[string]string{
335+
"Accept": "application/vnd.github.preview+json",
336+
},
337+
existingHeaders: map[string]string{
338+
"Accept": "application/json",
339+
},
340+
expectedHeaders: map[string]string{
341+
"Accept": "application/json,application/vnd.github.preview+json",
342+
},
343+
expectRoundTripCall: true,
344+
},
345+
{
346+
name: "preserve existing Accept application/octet-stream",
347+
previewHeaders: map[string]string{
348+
"Accept": "application/vnd.github.preview+json",
349+
},
350+
existingHeaders: map[string]string{
351+
"Accept": "application/octet-stream",
352+
},
353+
expectedHeaders: map[string]string{
354+
"Accept": "application/octet-stream",
355+
},
356+
expectRoundTripCall: true,
357+
},
358+
{
359+
name: "preserve existing accept application/octet-stream (lowercase)",
360+
previewHeaders: map[string]string{
361+
"accept": "application/vnd.github.preview+json",
362+
},
363+
existingHeaders: map[string]string{
364+
"accept": "application/octet-stream",
365+
},
366+
expectedHeaders: map[string]string{
367+
"Accept": "application/octet-stream",
368+
},
369+
expectRoundTripCall: true,
370+
},
371+
{
372+
name: "preserve existing Accept application/octet-stream (mixed case)",
373+
previewHeaders: map[string]string{
374+
"AcCePt": "application/vnd.github.preview+json",
375+
},
376+
existingHeaders: map[string]string{
377+
"Accept": "application/octet-stream",
378+
},
379+
expectedHeaders: map[string]string{
380+
"Accept": "application/octet-stream",
381+
},
382+
expectRoundTripCall: true,
383+
},
384+
{
385+
name: "multiple preview headers",
386+
previewHeaders: map[string]string{
387+
"Accept": "application/vnd.github.v3+json",
388+
"X-GitHub-Api-Version": "2022-11-28",
389+
},
390+
existingHeaders: map[string]string{},
391+
expectedHeaders: map[string]string{
392+
"Accept": "application/vnd.github.v3+json",
393+
"X-Github-Api-Version": "2022-11-28",
394+
},
395+
expectRoundTripCall: true,
396+
},
397+
{
398+
name: "append multiple preview headers to existing",
399+
previewHeaders: map[string]string{
400+
"Accept": "application/vnd.github.v3+json",
401+
"X-GitHub-Api-Version": "2022-11-28",
402+
},
403+
existingHeaders: map[string]string{
404+
"Accept": "application/json",
405+
"X-GitHub-Api-Version": "2021-01-01",
406+
},
407+
expectedHeaders: map[string]string{
408+
"Accept": "application/json,application/vnd.github.v3+json",
409+
"X-Github-Api-Version": "2021-01-01,2022-11-28",
410+
},
411+
expectRoundTripCall: true,
412+
},
413+
{
414+
name: "non-accept headers always append",
415+
previewHeaders: map[string]string{
416+
"X-Custom-Header": "preview-value",
417+
},
418+
existingHeaders: map[string]string{
419+
"X-Custom-Header": "application/octet-stream",
420+
},
421+
expectedHeaders: map[string]string{
422+
"X-Custom-Header": "application/octet-stream,preview-value",
423+
},
424+
expectRoundTripCall: true,
425+
},
426+
{
427+
name: "accept header with different value appends",
428+
previewHeaders: map[string]string{
429+
"Accept": "application/vnd.github.preview+json",
430+
},
431+
existingHeaders: map[string]string{
432+
"Accept": "application/json",
433+
},
434+
expectedHeaders: map[string]string{
435+
"Accept": "application/json,application/vnd.github.preview+json",
436+
},
437+
expectRoundTripCall: true,
438+
},
439+
}
440+
441+
for _, tt := range tests {
442+
t.Run(tt.name, func(t *testing.T) {
443+
// Create a mock RoundTripper that records the request
444+
var capturedRequest *http.Request
445+
mockRT := &mockRoundTripper{
446+
roundTripFunc: func(req *http.Request) (*http.Response, error) {
447+
capturedRequest = req
448+
return &http.Response{
449+
StatusCode: http.StatusOK,
450+
Body: http.NoBody,
451+
}, nil
452+
},
453+
}
454+
455+
injector := &previewHeaderInjectorTransport{
456+
rt: mockRT,
457+
previewHeaders: tt.previewHeaders,
458+
}
459+
460+
// Create a test request with existing headers
461+
req := httptest.NewRequest(http.MethodGet, "https://api.github.com/test", nil)
462+
for name, value := range tt.existingHeaders {
463+
req.Header.Set(name, value)
464+
}
465+
466+
// Execute RoundTrip
467+
resp, err := injector.RoundTrip(req)
468+
// Verify no error
469+
if err != nil {
470+
t.Fatalf("unexpected error: %v", err)
471+
}
472+
473+
// Verify response
474+
if resp == nil {
475+
t.Fatal("expected non-nil response")
476+
}
477+
478+
// Verify RoundTrip was called on the underlying transport
479+
if tt.expectRoundTripCall && capturedRequest == nil {
480+
t.Fatal("expected RoundTrip to be called on underlying transport")
481+
}
482+
483+
// Verify headers in the captured request
484+
if capturedRequest != nil {
485+
for name, expectedValue := range tt.expectedHeaders {
486+
actualValue := capturedRequest.Header.Get(name)
487+
if actualValue != expectedValue {
488+
t.Errorf("header %q: expected %q, got %q", name, expectedValue, actualValue)
489+
}
490+
}
491+
492+
// Verify no unexpected headers were added
493+
for name := range capturedRequest.Header {
494+
if _, exists := tt.expectedHeaders[name]; !exists {
495+
// Allow headers that were in existingHeaders but not in expectedHeaders
496+
if _, wasExisting := tt.existingHeaders[name]; !wasExisting {
497+
t.Errorf("unexpected header %q: %q", name, capturedRequest.Header.Get(name))
498+
}
499+
}
500+
}
501+
}
502+
})
503+
}
504+
}
505+
506+
// mockRoundTripper is a mock implementation of http.RoundTripper for testing.
507+
type mockRoundTripper struct {
508+
roundTripFunc func(*http.Request) (*http.Response, error)
509+
}
510+
511+
func (m *mockRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
512+
if m.roundTripFunc != nil {
513+
return m.roundTripFunc(req)
514+
}
515+
return &http.Response{StatusCode: http.StatusOK, Body: http.NoBody}, nil
516+
}

0 commit comments

Comments
 (0)