-
Notifications
You must be signed in to change notification settings - Fork 0
Added ability to pin the neoboard-sdk commit #121
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
base: main
Are you sure you want to change the base?
Changes from all commits
1b2ccb2
309667d
19e8d9c
5e4fe44
ad05a72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,10 @@ on: | |
| neoboard_ref: | ||
| description: 'NeoBoard Git ref to check out' | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: false | ||
|
|
||
| jobs: | ||
| build: | ||
| runs-on: ubuntu-latest | ||
|
|
@@ -25,18 +29,60 @@ jobs: | |
| steps: | ||
| - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 | ||
| with: | ||
| repository: nordeck/matrix-neoboard | ||
| path: matrix-neoboard | ||
| ref: ${{ github.events.inputs.neoboard_ref }} | ||
| path: matrix-neoboard-standalone | ||
| # required for changesets | ||
| fetch-depth: '0' | ||
| # don't persist the credentials so the changesets action doesn't use the | ||
| # github actions token but the git token provided via environment variable | ||
| persist-credentials: false | ||
|
|
||
| - name: Resolve SDK commit pin | ||
| id: sdk-pin | ||
| # Refresh on push to main (track latest upstream main) or on a manual | ||
| # workflow_dispatch run (track the given neoboard_ref instead, if any). | ||
| # Skipped for PR/other builds so they stay reproducible against | ||
| # whatever commit is actually checked into sdk.version. | ||
| working-directory: ./matrix-neoboard-standalone/ | ||
| env: | ||
| NEOBOARD_REF: ${{ github.event.inputs.neoboard_ref }} | ||
| run: | | ||
| RESOLVED_SHA=$(tr -d '[:space:]' < sdk.version) | ||
|
|
||
| if [[ -n "$NEOBOARD_REF" ]]; then | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This parameter existed before this PR, it was empty so it just checkout HEAD always, but you could enter your value. |
||
| # Explicit manual override always wins, regardless of changeset state. | ||
| RESOLVED_SHA=$(git ls-remote https://github.com/nordeck/matrix-neoboard.git "$NEOBOARD_REF" "$NEOBOARD_REF^{}" | awk 'END { print $1 }') | ||
| if [[ -z "$RESOLVED_SHA" ]]; then | ||
| # Not a named branch/tag on the remote - validate it as a commit SHA. | ||
| RESOLVED_SHA="$NEOBOARD_REF" | ||
| fi | ||
| echo "Manual override via neoboard_ref: using $RESOLVED_SHA." | ||
| elif [[ "${{ github.event_name }}" == "push" && "${{ github.ref }}" == "refs/heads/main" ]]; then | ||
| # A release PR merge has no pending changesets and must build from | ||
| # the pin committed in that release commit. Feature merges track | ||
| # the latest SDK and persist the resolved SHA in the release PR. | ||
| PENDING_CHANGESETS=$(find .changeset -maxdepth 1 -name '*.md' ! -name 'README.md' | wc -l) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can also merge a PR without changeset to main with no changesets, it will not be a release PR.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This usecase is covered in the "Renovate bot" section. Merges without a changeset won't create a new release. |
||
| if [[ "$PENDING_CHANGESETS" -gt 0 ]]; then | ||
| RESOLVED_SHA=$(git ls-remote https://github.com/nordeck/matrix-neoboard.git refs/heads/main | cut -f1) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is really strange I would say that we basically ignore the A merge commit will use latest from sdk, not from Just think about it: use a I think one of the main purposes of this PR is to make deterministic builds.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Release only happens when there are 0 changeset files and the project.json version is bumped. This is managed by that special PR the changeset bot maintains, sdk.version file is used then. Other times it's just being updated in the release PR automatically to follow the HEAD. |
||
| echo "Pending changesets found: using matrix-neoboard main at $RESOLVED_SHA." | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It also also updates "Version Packages" with latest sdk version, not with Or CI could suddenly just fail in "Version Packages". Additionally "Version Packages" are forced pushed by changesets, so if something is changed in "Version Packages" manually like
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's the idea, the sdk.version is getting updates on every merge to main. But when you merge the "Version Packages" to main, it's when it won't use HEAD, but the version from the file and a release will trigger. |
||
| else | ||
| echo "No pending changesets: using committed SDK pin $RESOLVED_SHA." | ||
| fi | ||
| else | ||
| echo "Using committed SDK pin $RESOLVED_SHA." | ||
| fi | ||
|
|
||
| if [[ ! "$RESOLVED_SHA" =~ ^[0-9a-fA-F]{40}$ ]]; then | ||
| echo "::error::SDK revision must be a full 40-character commit SHA, got '$RESOLVED_SHA'." | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "sha=$RESOLVED_SHA" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 | ||
| with: | ||
| path: matrix-neoboard-standalone | ||
| repository: nordeck/matrix-neoboard | ||
| path: matrix-neoboard | ||
| ref: ${{ steps.sdk-pin.outputs.sha }} | ||
| # required for changesets | ||
| fetch-depth: '0' | ||
| # don't persist the credentials so the changesets action doesn't use the | ||
|
|
@@ -104,15 +150,19 @@ jobs: | |
|
|
||
| - name: get argo-cd type of sha | ||
| id: argocd-sha | ||
| env: | ||
| SDK_SHA: ${{ steps.sdk-pin.outputs.sha }} | ||
| run: | | ||
| COMMIT_SHA=${{ github.sha }} | ||
| if [[ '${{ github.event_name }}' == 'pull_request' ]] | ||
| then | ||
| COMMIT_SHA=${{ github.event.pull_request.head.sha }} | ||
| fi | ||
| ARGOCD_TAG=argo-$(echo $COMMIT_SHA | cut -c1-8) | ||
| COMPOSITE_TAG=standalone-${COMMIT_SHA}-sdk-${SDK_SHA} | ||
|
segnord marked this conversation as resolved.
|
||
| echo "ARGOCD_TAG=$ARGOCD_TAG" >> $GITHUB_ENV | ||
| echo "ARGOCD_TAG=$ARGOCD_TAG" >> "$GITHUB_OUTPUT" | ||
| echo "COMPOSITE_TAG=$COMPOSITE_TAG" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Generate Docker metadata | ||
| id: meta | ||
|
|
@@ -128,6 +178,7 @@ jobs: | |
| tags: | | ||
| type=sha,prefix= | ||
| type=raw,value=${{ env.ARGOCD_TAG }} | ||
| type=raw,value=${{ steps.argocd-sha.outputs.COMPOSITE_TAG }} | ||
|
|
||
| # Read the created files into variables | ||
| - name: Read version and revision to be used by docker | ||
|
|
@@ -190,13 +241,21 @@ jobs: | |
| private_key: ${{ secrets.GH_APP_OS_PRIVATE_KEY }} | ||
|
|
||
| - name: Create Release Pull Request or Publish Packages | ||
| # This action creates/updates the "Version Packages" pull request. | ||
| # It has 2 outcomes depending if the workspace contains any changeset files and package.json version. | ||
| # changeset files exist (version): Creates/updates the "Version packages" pull request. | ||
| # Bumps the package.json version. | ||
| # Writes the SDK used by this build to sdk.version. | ||
| # no changeset files (publish): tags the current commit with the new semver. | ||
|
Comment on lines
+244
to
+249
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I read it if feels confusing, things are mixed up together: "Version Packages", "2 outcomes", "any", "and". It makes to to feel like you have to check what it does and there are just few command, maybe could improve.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's just the changesets github action has version and publish outcomes depending on the kind of PR is merged, that may not be obvious at a glance. |
||
| if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} | ||
| uses: changesets/action@a45c4d594aa4e2c509dc14a9f2b3b67ba3780d0d # v1.9.0 | ||
| with: | ||
| version: yarn changeset:version | ||
|
segnord marked this conversation as resolved.
|
||
| publish: yarn changeset tag | ||
| cwd: ./matrix-neoboard-standalone/ | ||
| env: | ||
| GITHUB_TOKEN: ${{ steps.generate_token.outputs.token }} | ||
| NEOBOARD_SDK_SHA: ${{ steps.sdk-pin.outputs.sha }} | ||
|
|
||
| helm-lint-test: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| # Neoboard React SDK pinning | ||
|
|
||
| This explains which commit of [`matrix-neoboard`](https://github.com/nordeck/matrix-neoboard)'s `react-sdk` | ||
| is used to build it in different scenarios. | ||
|
|
||
| The pinned commit lives in [`sdk.version`](../sdk.version) at the repo root. | ||
| Whether it gets refreshed before a build - and what ends up tagged - depends on the trigger. | ||
|
Comment on lines
+6
to
+7
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the whole document actually shows how confusingly
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The release process isn't trivial, the scripts try keep up with sdk main branch automatically while freezing the version predictably for a release with frozen commit hashes. |
||
|
|
||
| ## Scenarios | ||
|
|
||
| | Scenario | Trigger | Docker image built using | SDK commit used | What (if anything) gets tagged | | ||
| | ------------------------------------------------------- | --------------------------------------------------------------------------------------------------- | --------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | **Push to a feature branch (no pull request open yet)** | _(none - `ci.yml` only reacts to `push` on `main`/`release/*`, `pull_request`, or manual dispatch)_ | - | - | - | | ||
| | **Feature PR open/updated** | `pull_request` | **`sdk.version` pin** (as committed on that branch) | Not refreshed - whatever is already committed, so PR builds are reproducible across reruns. | When the image is pushed, `standalone-<PR head SHA>-sdk-<SDK SHA>`. | | ||
| | **Feature PR merged to `main`** | `push` to `main`, with pending changesets | **HEAD of `matrix-neoboard` `main`** | Resolved once before the build and passed to the custom Changesets version command, which writes the same SHA to `sdk.version` after preparing the "Version Packages" release branch. | `standalone-<standalone SHA>-sdk-<SDK SHA>`. | | ||
| | **Release PR merged to `main`** | `push` to `main`, no pending changesets left | **`sdk.version` pin** (as committed in that merge commit) | Deliberately **not** re-refreshed against HEAD, so the build always matches its own git history exactly - even if upstream has moved further since the release PR was opened and updated. | The CI build receives `standalone-<standalone SHA>-sdk-<SDK SHA>`. After `yarn changeset tag` creates `vX.Y.Z`, `publish-release.yml` also promotes it as `standalone-X.Y.Z-sdk-<SDK SHA>`. | | ||
|
|
||
| ## Rule of thumb | ||
|
|
||
| The build tracks **HEAD** only when a change is actually going to be captured in | ||
| an upcoming release PR (so nothing is lost). The SHA is resolved once and used | ||
| both for the build and the `changeset:version` command. Because Changesets prepares | ||
| its release branch before running that command, the generated release commit records | ||
| the exact SDK revision that was built. | ||
|
|
||
| The workflow always falls back to the **pin** whenever a build's result could | ||
| otherwise be tagged/released without a corresponding commit - PR builds | ||
| (which are only pushed when package credentials are available) and release-PR-merge builds | ||
| (this is the exact commit about to be tagged) - so a release can never contain | ||
| an SDK commit that isn't also recorded in `sdk.version`'s own git history. | ||
|
|
||
| ## Container tags | ||
|
|
||
| Every image pushed by `ci.yml` receives a composite revision tag: | ||
|
|
||
| ```text | ||
| standalone-<standalone commit SHA>-sdk-<SDK commit SHA> | ||
| ``` | ||
|
|
||
| Release promotion keeps the existing semantic-version tag and also adds: | ||
|
|
||
| ```text | ||
| standalone-<standalone version>-sdk-<SDK commit SHA> | ||
| ``` | ||
|
|
||
| The existing SHA, Argo CD, semantic-version, and `latest` tags remain available | ||
| for existing consumers. Both composite tags use the SDK commit selected for the | ||
| original CI build; released SDK package versions are not used. | ||
|
|
||
| ## Other triggers | ||
|
|
||
| Two additional triggers exist but are out of scope of the scenarios above: | ||
|
|
||
| - Pushes to `release/*` branches always build from the pin - both the refresh step | ||
| and the changesets/tagging logic are skipped entirely there. | ||
| - A manual `workflow_dispatch` run can override with an explicit `neoboard_ref` | ||
| input, which always takes precedence over both the pin and HEAD. | ||
|
|
||
| ## Renovate Bot PRs | ||
|
|
||
| Renovate's PRs (dependency bumps, Docker/action digest updates, etc.) merge to | ||
| `main` without adding a changeset file. As long as no other changeset happens to | ||
| be pending at the same time, that merge lands in the same "no pending changesets" | ||
| state as the **Release PR merged to `main`** row above - it's not specific to | ||
| release PRs, it's whatever the changeset state happens to be at merge time. | ||
|
|
||
| This has two consequences: | ||
|
|
||
| - **No new release is triggered.** `changesets/action` finds nothing to version, | ||
| so it goes straight to the `publish` path and runs `yarn changeset tag`. That looks | ||
| for a tag matching the current `package.json` version, finds it already exists from | ||
| the last real release, and skips creating a duplicate - so the merge produces no | ||
| new tag and `publish-release.yml` never fires. | ||
| - **The build uses the pinned `sdk.version`, not HEAD.** Since there are no | ||
| pending changesets, the "Update SDK commit pin" step skips refreshing against | ||
| upstream `matrix-neoboard` and builds from whatever commit is already committed | ||
| in `sdk.version` - exactly the same reasoning as the release-PR-merge case: | ||
| this commit could in principle end up being what a release points to, so the | ||
| build must match its own git history rather than a freshly-resolved HEAD. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| "depcheck": "depcheck --ignores=@vitest/coverage-v8,@changesets/cli,@swc/core,@testing-library/dom,@testing-library/jest-dom,@testing-library/react,@testing-library/react-hooks,@typescript-eslint/eslint-plugin,@typescript-eslint/parser,prettier-plugin-organize-imports,identity-obj-proxy", | ||
| "deduplicate": "yarn-deduplicate", | ||
| "translate": "i18next-cli extract --sync-primary", | ||
| "changeset:version": "yarn changeset version && node scripts/update-sdk-version.mjs", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Initially the PR didn't have it and it just wrote the SHA to file. The new script contains a SHA structure verification. @mgcm may explain better. |
||
| "generate-disclaimer": "cp LICENSE ./build/LICENSE.txt && cp NOTICE ./build/NOTICE.txt && yarn licenses generate-disclaimer --prod >> ./build/NOTICE.txt && yarn licenses list --prod --json --no-progress > ./build/licenses.json", | ||
| "docker:build": "sh -c \"${CONTAINER_CLI:-docker} build -t nordeck/matrix-neoboard-standalone .\"", | ||
| "docker:run": "sh -c \"${CONTAINER_CLI:-docker} run -p 8888:8080 --rm --name matrix-neoboard-standalone nordeck/matrix-neoboard-standalone\"", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| /* | ||
| * Copyright 2026 Nordeck IT + Consulting GmbH | ||
| * | ||
| * NeoBoard Standalone is free software: you can redistribute it and/or modify | ||
| * it under the terms of the GNU Affero General Public License as published by | ||
| * the Free Software Foundation, either version 3 of the License, or (at your | ||
| * option) any later version. | ||
| * | ||
| * NeoBoard Standalone is distributed in the hope that it will be useful, but | ||
| * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY | ||
| * or FITNESS FOR A PARTICULAR PURPOSE. | ||
| * | ||
| * See the GNU Affero General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU Affero General Public License | ||
| * along with NeoBoard Standalone. If not, see <https://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
| import { writeFile } from 'node:fs/promises'; | ||
| import process from 'node:process'; | ||
| import { URL } from 'node:url'; | ||
|
|
||
| const sdkSha = process.env.NEOBOARD_SDK_SHA; | ||
|
|
||
| if (!sdkSha || !/^[0-9a-fA-F]{40}$/.test(sdkSha)) { | ||
| throw new Error( | ||
| 'NEOBOARD_SDK_SHA must contain a full 40-character commit SHA.', | ||
| ); | ||
| } | ||
|
|
||
| await writeFile(new URL('../sdk.version', import.meta.url), `${sdkSha}\n`); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| 67758ec8c804e5a2aa3be5b6e0810e45b51dabc8 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feels fine to store it in file. But I think file should be better named,
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a sdk_version.sh file so the naming followed it. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's consider to use
sdk.versionalways and don't try to magically replace/refresh it's value to latest in sdk. I think this could remove all the confusion and get things easy to understand. Anyone can just updatesdk.versionin any PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was to pin the commit only during release and never think about manually tracking the HEAD of the SDK repo during feature commits to main (same workflow as befofore this PR).