-
Notifications
You must be signed in to change notification settings - Fork 1
feat(download): v2 CI harness + minimal ListDatasets #681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
c338612
docs: pin neicnordic/sensitive-data-archive commit for v2 integration CI
jhagberg 7d79a43
feat(apiclient): v2 wire types (v2File, list responses, DatasetInfo)
jhagberg 534fee0
feat(apiclient): V2Client with minimal ListDatasets
jhagberg 27e1f94
refactor(apiclient): clarify Config.Version comment + trim pagination…
jhagberg b0a42de
ci: v2 integration tests against pinned SDA dev stack
jhagberg da70a40
fix(ci): harden v2 integration workflow
jhagberg 2d7a835
fix(list): restore legacy v1 error-wrap prefix for list commands
jhagberg 779d968
refactor(downloadclient): address PR review feedback from #681
jhagberg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| name: integration-v2 | ||
|
|
||
| on: | ||
| pull_request: | ||
| push: | ||
| branches: [main] | ||
|
|
||
| jobs: | ||
| v2-integration: | ||
| runs-on: ubuntu-22.04 | ||
| steps: | ||
| - name: Checkout sda-cli | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| path: sda-cli | ||
|
|
||
| - name: Checkout sensitive-data-archive (pinned) | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| repository: neicnordic/sensitive-data-archive | ||
| ref: 608878fa453770fcb3962bf0239366905c125982 # bump in docs/v2-dev-stack-pin.md when the contract changes | ||
| path: sensitive-data-archive | ||
|
|
||
| - name: Set up Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version-file: sda-cli/go.mod | ||
|
|
||
| - name: Boot v2 dev stack | ||
| working-directory: sensitive-data-archive | ||
| run: | | ||
| make build-all | ||
| PR_NUMBER=$(date +%F) make dev-download-v2-up | ||
|
|
||
| - name: Wait for services | ||
| run: | | ||
| for i in {1..120}; do | ||
| if curl -sf http://localhost:8085/health/ready \ | ||
| && curl -sf http://localhost:8000/tokens > /dev/null; then | ||
| echo "download service and mockauth ready" | ||
| exit 0 | ||
| fi | ||
| sleep 2 | ||
| done | ||
| echo "services did not come up in 240s" | ||
| exit 1 | ||
|
|
||
| - name: Fetch dev token | ||
| id: token | ||
| run: | | ||
| TOKEN=$(curl -s http://localhost:8000/tokens | jq -r '.[0]') | ||
| if [ -z "$TOKEN" ] || [ "$TOKEN" = "null" ]; then | ||
| echo "failed to fetch dev token" | ||
| exit 1 | ||
| fi | ||
| echo "::add-mask::$TOKEN" | ||
| echo "TOKEN=$TOKEN" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Run v2 integration tests | ||
| working-directory: sda-cli | ||
| env: | ||
| DOWNLOAD_V2_URL: http://localhost:8085 | ||
| DOWNLOAD_V2_TOKEN: ${{ steps.token.outputs.TOKEN }} | ||
| run: | | ||
| go test -tags=integration -v ./... | ||
|
|
||
| - name: Dump all service logs on failure | ||
| if: failure() | ||
| working-directory: sensitive-data-archive | ||
| run: | | ||
| docker compose --project-name download-v2-dev logs || true | ||
|
|
||
| - name: Tear down (always) | ||
| if: always() | ||
| working-directory: sensitive-data-archive | ||
| run: PR_NUMBER=$(date +%F) make dev-download-v2-down || true |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # SDA v2 dev-stack pin | ||
|
|
||
| Integration tests for `--api-version v2` boot the SDA v2 dev stack from | ||
| `neicnordic/sensitive-data-archive` at the commit below. Bump when the v2 | ||
| server contract changes or when features we depend on land in main. | ||
|
|
||
| **Current pin:** `608878fa453770fcb3962bf0239366905c125982` | ||
| **Updated:** 2026-04-22 | ||
| **Why this commit:** Latest `origin/main` commit that directly touched | ||
| `dev-tools/download-v2-dev/` — specifically, the final round of Copilot | ||
| review fixes on the dev-compose that was introduced in | ||
| neicnordic/sensitive-data-archive#2368 | ||
| (`feat(download): add lightweight dev compose for v2 API`). Pinning here | ||
| gives us the dev stack in its reviewed-and-stabilized shape; later | ||
| `origin/main` commits change unrelated areas (e.g. readiness probes, | ||
| s3inbox) that could shift CI behavior without touching the file we depend | ||
| on. | ||
|
|
||
| ## Bumping | ||
|
|
||
| 1. Read the diff: `git log <old>..<new> -- sda/cmd/download/ dev-tools/download-v2-dev/` | ||
| 2. Run the pinned commit locally: `git checkout <new> && make dev-download-v2-up && go test -tags integration ./...` | ||
| 3. If tests pass, update `.github/workflows/integration-v2.yml` and this file in the same commit. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| //go:build integration | ||
|
|
||
| package download_test | ||
|
|
||
| import ( | ||
| "context" | ||
| "os" | ||
| "testing" | ||
|
|
||
| "github.com/NBISweden/sda-cli/downloadclient" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestV2_ListDatasets_Smoke calls the real v2 dev stack. Requires: | ||
| // - dev-tools/download-v2-dev/ stack is up (make dev-download-v2-up) | ||
| // - DOWNLOAD_V2_URL env var (default http://localhost:8085) | ||
| // - DOWNLOAD_V2_TOKEN env var (dev token from mockauth) | ||
| func TestV2_ListDatasets_Smoke(t *testing.T) { | ||
| baseURL := os.Getenv("DOWNLOAD_V2_URL") | ||
| if baseURL == "" { | ||
| baseURL = "http://localhost:8085" | ||
| } | ||
| token := os.Getenv("DOWNLOAD_V2_TOKEN") | ||
| require.NotEmpty(t, token, "DOWNLOAD_V2_TOKEN must be set (curl /tokens on mockauth)") | ||
|
|
||
| client, err := downloadclient.New(downloadclient.Config{ | ||
| BaseURL: baseURL, | ||
| Token: token, | ||
| ClientVersion: "test", | ||
| }, "v2") | ||
| require.NoError(t, err) | ||
|
|
||
| got, err := client.ListDatasets(context.Background()) | ||
| require.NoError(t, err) | ||
| assert.Contains(t, got, "EGAD00000000001", "dev stack should expose seeded dataset EGAD00000000001") | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| package downloadclient | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "io" | ||
| "net/http" | ||
| "net/url" | ||
| ) | ||
|
|
||
| // maxErrorBodyBytes is the maximum number of bytes from a server error | ||
| // response that getJSON includes in its formatted error. Bodies past this | ||
| // are truncated; #678 caps the read itself with io.LimitReader on the | ||
| // response body to bound memory before truncation. | ||
| const maxErrorBodyBytes = 200 | ||
|
|
||
| // V2Client talks to the v2 SDA download API | ||
| // (GET /datasets, /datasets/{id}/files, /files/{id}, etc.). | ||
| // Methods fill in across #675, #676, #677. Until then, unimplemented | ||
| // methods return a clear "not implemented until #N" error. | ||
| type V2Client struct { | ||
| cfg Config | ||
| http *http.Client | ||
| } | ||
|
|
||
| // NewV2Client constructs a V2Client. HTTP client is a plain net/http.Client | ||
| // (no cookie jar — v2 is stateless bearer-token auth, no Location-based | ||
| // redirects requiring cookies). | ||
| func NewV2Client(cfg Config) *V2Client { | ||
| return &V2Client{ | ||
| cfg: cfg, | ||
| http: &http.Client{}, | ||
| } | ||
| } | ||
|
|
||
| // ListDatasets implements Client. Single-page only here; pagination via the | ||
| // paginate[T] helper arrives in #676. Returning an explicit error when | ||
| // nextPageToken != null prevents silently truncating results. | ||
| func (c *V2Client) ListDatasets(ctx context.Context) ([]string, error) { | ||
| endpoint, err := url.JoinPath(c.cfg.BaseURL, "datasets") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("invalid base URL: %w", err) | ||
| } | ||
| body, err := c.getJSON(ctx, endpoint) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer body.Close() //nolint:errcheck | ||
|
|
||
| var resp datasetListResponse | ||
| if err := json.NewDecoder(body).Decode(&resp); err != nil { | ||
| return nil, fmt.Errorf("failed to decode /datasets response: %w", err) | ||
| } | ||
| if resp.NextPageToken != nil && *resp.NextPageToken != "" { | ||
| return nil, errors.New("pagination not yet implemented (coming in #676)") | ||
| } | ||
|
|
||
| return resp.Datasets, nil | ||
| } | ||
|
|
||
| // ListFiles implements Client. Not implemented until #676. | ||
| func (c *V2Client) ListFiles(_ context.Context, _ string, _ ListFilesOptions) ([]File, error) { | ||
| return nil, errors.New("V2Client.ListFiles not implemented until #676") | ||
| } | ||
|
|
||
| // DatasetInfo implements Client. Not implemented until #676. | ||
| func (c *V2Client) DatasetInfo(_ context.Context, _ string) (DatasetInfo, error) { | ||
| return DatasetInfo{}, errors.New("V2Client.DatasetInfo not implemented until #676") | ||
| } | ||
|
|
||
| // getJSON performs an authenticated GET returning the response body. | ||
| // #678 replaces error wrapping with RFC 9457 Problem Details parsing. | ||
| func (c *V2Client) getJSON(ctx context.Context, reqURL string) (io.ReadCloser, error) { | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("build request: %w", err) | ||
| } | ||
| req.Header.Set("Authorization", "Bearer "+c.cfg.Token) | ||
| req.Header.Set("Accept", "application/json") | ||
| if c.cfg.ClientVersion != "" { | ||
| req.Header.Set("User-Agent", "sda-cli/"+c.cfg.ClientVersion) | ||
| req.Header.Set("SDA-Client-Version", c.cfg.ClientVersion) | ||
| } | ||
|
|
||
| resp, err := c.http.Do(req) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("http request: %w", err) | ||
| } | ||
| if resp.StatusCode < 200 || resp.StatusCode >= 300 { | ||
| b, _ := io.ReadAll(resp.Body) | ||
| _ = resp.Body.Close() | ||
| body := string(b) | ||
| if len(body) > maxErrorBodyBytes { | ||
| body = body[:maxErrorBodyBytes] | ||
| } | ||
|
|
||
| return nil, fmt.Errorf("server returned status %d: %s", resp.StatusCode, body) | ||
| } | ||
|
|
||
| return resp.Body, nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package downloadclient | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestV2Client_ListDatasets_SinglePage(t *testing.T) { | ||
| ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| assert.Equal(t, "/datasets", r.URL.Path) | ||
| assert.Equal(t, "Bearer v2-token", r.Header.Get("Authorization")) | ||
| w.Header().Set("Content-Type", "application/json") | ||
| fmt.Fprint(w, `{"datasets":["EGAD00000000001","EGAD00000000002"],"nextPageToken":null}`) | ||
| })) | ||
| defer ts.Close() | ||
|
|
||
| c := NewV2Client(Config{BaseURL: ts.URL, Token: "v2-token"}) | ||
| c.http = ts.Client() | ||
|
|
||
| got, err := c.ListDatasets(context.Background()) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, []string{"EGAD00000000001", "EGAD00000000002"}, got) | ||
| } | ||
|
|
||
| func TestV2Client_ListDatasets_MultiPageNotYetImplemented(t *testing.T) { | ||
| // Minimal ListDatasets returns the first page only. If nextPageToken | ||
| // is non-null, we explicitly warn by returning an error so #676 can | ||
| // flip this into a real paginate call without silent truncation. | ||
| ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| fmt.Fprint(w, `{"datasets":["EGAD001"],"nextPageToken":"ptk_second"}`) | ||
| })) | ||
| defer ts.Close() | ||
|
|
||
| c := NewV2Client(Config{BaseURL: ts.URL, Token: "t"}) | ||
| c.http = ts.Client() | ||
|
|
||
| _, err := c.ListDatasets(context.Background()) | ||
| require.Error(t, err) | ||
| assert.Contains(t, err.Error(), "pagination not yet implemented") | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.