Skip to content

Add encoding outputs create gcs-service-account command#10

Open
dweinber wants to merge 1 commit into
mainfrom
feat/outputs-create-gcs-service-account
Open

Add encoding outputs create gcs-service-account command#10
dweinber wants to merge 1 commit into
mainfrom
feat/outputs-create-gcs-service-account

Conversation

@dweinber
Copy link
Copy Markdown
Member

Summary

Add `bitmovin encoding outputs create gcs-service-account` to wrap `POST /v1/encoding/outputs/gcs-service-account`.

The CLI previously only exposed HMAC-based GCS outputs (`encoding outputs create gcs`), even though the API supports both HMAC and Service Account-based GCS outputs. Service-account-based outputs are the recommended path on GCP — HMAC keys are S3-compat shims that don't always handle every supported feature, and many organizations disable HMAC interoperability via org policy entirely.

Usage

```zsh
bitmovin encoding outputs create gcs-service-account \
--name my-output \
--bucket my-bucket \
--service-account-key-file ./sa-key.json \
--cloud-region EUROPE_WEST_1
```

Changes

  • New command at `src/commands/encoding/outputs/create/gcs-service-account.ts`.
  • Reads the SA key file directly via `--service-account-key-file` so credentials never appear in shell history.
  • Validates the file is parseable JSON before calling the API, for a clearer error path than the SDK's downstream rejection.
  • Optional `--cloud-region` flag, matching the existing GCS HMAC command shape.

Tests

  • New tests cover the success path (creating an output with credentials read from a temp file), invalid-JSON file, and missing file.
  • All 111 tests pass.
  • Verified end-to-end against the real API: creates the output, the SDK returns the expected `GCS_SERVICE_ACCOUNT` output object.

Test plan

  • CI green
  • `bitmovin encoding outputs create gcs-service-account ...` creates an output
  • Invalid JSON / missing key file fail before reaching the API

Add `bitmovin encoding outputs create gcs-service-account` to wrap
POST /v1/encoding/outputs/gcs-service-account. Service-account-based
GCS outputs use the SDK's typed `GcsServiceAccountOutput` model with
the JSON key file content; previously, only HMAC-based GCS outputs
were exposed in the CLI even though the API supports both.

Reads the JSON key file directly via --service-account-key-file so
credentials never appear in shell history. Validates the file is
parseable JSON before calling the API for a clearer error path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@lukaskroepfl
Copy link
Copy Markdown
Member

Solid — fills a real gap (HMAC-only GCS outputs were the awkward path on GCP), reads the SA file directly so credentials don't end up in shell history, validates JSON before the API call, and the tests cover the three meaningful states. Two things I'd change before merge:

  1. as never cast on cloudRegion: ...(flags['cloud-region'] && {cloudRegion: flags['cloud-region'] as never}) silences a real type mismatch. The SDK's GcsServiceAccountOutput.cloudRegion is an enum (presumably CloudRegion.EUROPE_WEST_1 etc.), but the flag accepts any string and casts to never. A user typo (EUROPE-WEST-1, europe_west_1, eu-west-1) reaches the API as-is and gets rejected with a less actionable error. Two ways to fix:

    • Define the flag with options: Object.values(CloudRegion) (the SDK should export the enum) so oclif rejects unknown values at parse time. This is what the existing gcs command does if it follows the pattern, worth copying.
    • At minimum, drop the as never and let TS narrow cloudRegion to the enum type so the compiler tells you next time the SDK changes the shape.
  2. No CHANGELOG entry. Recent PRs (Fix encoding templates create to send raw YAML body #5, Mask secrets in account info by default #6, Resolve API key from env var in config show #7, Add encoding jobs live command #9, Add CI workflow that packs tarballs and installer artifacts #11) all added [Unreleased] / Added entries; this one is user-visible (new command surface) and should match. One line under Added.

Smaller note

  • Tests use require('node:fs') etc. inside the describe. The rest of the repo's tests use ESM import at the top. Not wrong, just stylistically off — convert to top-level imports while you're in there.

Otherwise LGTM in principle once (1) and (2) are addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants