Skip to content

GH-49537: [C++][FlightRPC] Windows CI to Support ODBC DLL & MSI Signing#49603

Merged
amoeba merged 1 commit into
apache:mainfrom
Bit-Quill:gh-49537-ci-sign-win-odbc
Jun 9, 2026
Merged

GH-49537: [C++][FlightRPC] Windows CI to Support ODBC DLL & MSI Signing#49603
amoeba merged 1 commit into
apache:mainfrom
Bit-Quill:gh-49537-ci-sign-win-odbc

Conversation

@alinaliBQ

@alinaliBQ alinaliBQ commented Mar 26, 2026

Copy link
Copy Markdown
Collaborator

Rationale for this change

GH-49537

What changes are included in this PR?

  • Implement Windows CIs to:
    1. CI A - odbc-msvc-upload-dll Upload unsigned DLL
    2. CI B - odbc-msvc-upload-msi Download signed DLL and upload unsigned MSI
  • Remove odbc-release CI that is replaced by the new CIs.
  • Use composite action to reuse code for building ODBC Windows.
  • The release manager can run CI A first, then download the unsigned DLL and upload signed DLL.
  • Run CI B to upload unsigned MSI. Then sign the MSI locally, and upload signed MSI to GitHub release. Example Command to trigger CI B:
gh workflow run cpp_extra.yml --ref apache-arrow-test-24.0.0-rc0 -f odbc_release_step=true

Example of 07-flightsqlodbc-upload.sh script (not tested):
We need to either 1) implement a way to get RUN_ID and then call gh run watch,
or 2) enter each command manually and wait for the CI to finish.

# download unsigned DLL
gh release download $tag --pattern arrow_flight_sql_odbc_unsigned.dll

# sign ODBC DLL and upload to GitHub release
jsign arrow_flight_sql_odbc_unsigned.dll ...
mv arrow_flight_sql_odbc_unsigned.dll arrow_flight_sql_odbc.dll
gh release upload $tag --clobber arrow_flight_sql_odbc.dll

# trigger CI B
gh workflow run cpp_extra.yml --ref $tag -f odbc_upload=msi

# download unsigned MSI
gh release download $tag --pattern Apache-Arrow-Flight-SQL-ODBC-*-win64.msi

# sign ODBC MSI and upload to GitHub release
jsign Apache-Arrow-Flight-SQL-ODBC-*-win64.msi ...
gh release upload $tag --clobber Apache-Arrow-Flight-SQL-ODBC-*-win64.msi

# remove ODBC DLLs from GitHub release
gh release delete-asset $tag arrow_flight_sql_odbc_unsigned.dll --yes
gh release delete-asset $tag arrow_flight_sql_odbc.dll --yes

Documentation is in:

# Sign, build the installer for, and sign the installer for the FlightSQL
# ODBC Windows driver
#
# NOTE: This must be run by a PMC member
# Note: You need to have jsign installed and an available credential from
# ASF to sign artifacts. Not all PMC members will have access to code
# signing.
# Note: The script requires setup of ssl.com environment variables.
# Note: Invoking this script costs money.
dev/release/07-flightsqlodbc-upload.sh <version> <rc-number>

Are these changes tested?

  • The uploading and signing process is tested in my repo
  • Workflows are verified in CI

Are there any user-facing changes?

N/A

Comment thread .github/workflows/cpp_extra.yml Outdated
@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Mar 26, 2026
@alinaliBQ

Copy link
Copy Markdown
Collaborator Author

@amoeba PR is ready for review. We have an issue for flight-sql-tests ODBC Windows failure: #49465

@alinaliBQ
alinaliBQ marked this pull request as ready for review March 27, 2026 17:35
@alinaliBQ

Copy link
Copy Markdown
Collaborator Author

I did an empty commit (a522393) and got this error:

Error: D:\a\arrow\arrow\./.github/actions/odbc-windows\action.yml (Line: 71, Col: 12): Unrecognized named-value: 'secrets'. Located at position 1 within expression: secrets.GITHUB_TOKEN
Error: GitHub.DistributedTask.ObjectTemplating.TemplateValidationException: The template is not valid. D:\a\arrow\arrow\./.github/actions/odbc-windows\action.yml (Line: 71, Col: 12): Unrecognized named-value: 'secrets'. Located at position 1 within expression: secrets.GITHUB_TOKEN
   at GitHub.DistributedTask.ObjectTemplating.TemplateValidationErrors.Check()
   at GitHub.Runner.Worker.ActionManifestManagerLegacy.ConvertRuns(IExecutionContext executionContext, TemplateContext templateContext, TemplateToken inputsToken, String fileRelativePath, MappingToken outputs)
   at GitHub.Runner.Worker.ActionManifestManagerLegacy.Load(IExecutionContext executionContext, String manifestFile)
Error: Failed to load D:\a\arrow\arrow\./.github/actions/odbc-windows\action.yml

The same implementation worked yesterday (see https://github.com/apache/arrow/actions/runs/23622018872/job/68803175375). Seems that GitHub might have updated runner, I will look into this.

@alinaliBQ

Copy link
Copy Markdown
Collaborator Author

Error: Failed to load D:\a\arrow\arrow./.github/actions/odbc-windows\action.yml

This issue should be resolved now by commit 95bc75b

@alinaliBQ

Copy link
Copy Markdown
Collaborator Author

@kou @raulcd This PR is ready for review. Please have a look if you have a chance, thank you

Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml Outdated
@github-actions github-actions Bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting committer review Awaiting committer review awaiting changes Awaiting changes labels Apr 1, 2026

@alinaliBQ alinaliBQ left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed comments. Please have another look, thanks!

Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/cpp_extra.yml Outdated

@amoeba amoeba left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @alinaliBQ.

@raulcd do you want to have a look? Once we merge, I'll test the whole flow with the script (PR to come) that does the signing so we'll have more chances to fix any issues.

@raulcd raulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the work @alinaliBQ ! I am unsure this is ready yet, sorry it took me a couple days for review, it was Easter and had some family around. My main concern is around, the tag hasn't been updated to match the one on the packaging linux jobs, currently any tag will trigger the job and we only want to do it for RC's. The worlflow dispatch is still around and I thought this was not necessary with the tag.
@amoeba I am about to do the feature freeze, I think this is going to miss 24.0.0, is that ok?

Comment thread .github/workflows/cpp_extra.yml Outdated
schedule:
- cron: |
0 0 * * *
workflow_dispatch:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not sure I understand this. Didn't we agreed to use the tag for the RC? Why is the workflow dispatch needed and referenced below?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The flow is a bit complicated, we need to trigger some jobs when the RC is created but the odbc-msvc-upload-msi step needs to be triggered manually so we use workflow_dispatch. The sequence is,

  1. Release manager tags release
  2. cpp_extra.yml jobs odbc-msvc-upload-dll and odbc-dll-release are run automatically, unsigned DLL is added to release
  3. Release manager locally runs script (TBD) that downloads, signs (w/ jsign), and uploads the signed DLL to the release
  4. Release manager triggers odbc-msvc-upload-msi via workflow_dispatch which builds an unsigned MSI with a signed DLL inside and adds it to the release
  5. Release manager locally runs script that downloads signs (w/ jsign), and uploads the signed MSI to the releasee

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see, I did not understood the process. As the process isn't entirely automated and it requires several manual steps, can we document it on the release page?
https://github.com/apache/arrow/blob/main/docs/source/developers/release.rst

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@amoeba Could you kindly help with the documentation on release page?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Absolutely. I'll file an issue and fill it in as we finish the final PRs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

hi @raulcd, @amoeba has helped to document the process on release page:

# Sign, build the installer for, and sign the installer for the FlightSQL
# ODBC Windows driver
#
# NOTE: This must be run by a PMC member
# Note: You need to have jsign installed and an available credential from
# ASF to sign artifacts. Not all PMC members will have access to code
# signing.
# Note: The script requires setup of ssl.com environment variables.
# Note: Invoking this script costs money.
dev/release/07-flightsqlodbc-upload.sh <version> <rc-number>

@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Apr 7, 2026
@amoeba

amoeba commented Apr 7, 2026

Copy link
Copy Markdown
Member

Thanks for taking a look @raulcd. It's okay if this doesn't make it for 24 but if it's at all possible that'd be great.

Re: the jobs running on any tag, I'm not seeing what you see, could you point out which job needs updating?

  • odbc-msvc: Runs as normal (when labeled with "Extra" label). This is the pre-existing CI for the driver.
  • odbc-msvc-upload-dll: Only runs when ref matches an RC tag and correctly doesn't run on workflow_dispatch
  • odbc-dll-release: Only runs after odbc-msvc-upload-dll
  • odbc-msvc-upload-msi: Only runs on workflow_dispatch

I could see making the last one (odbc-msvc-upload-msi) more defensive.

@alinaliBQ

Copy link
Copy Markdown
Collaborator Author

I am unsure this is ready yet, sorry it took me a couple days for review, it was Easter and had some family around. My main concern is around, the tag hasn't been updated to match the one on the packaging linux jobs, currently any tag will trigger the job and we only want to do it for RC's.

No worries @raulcd, hope you had a good Easter.
odbc-msvc-upload-dll is the job that is only triggered by RC tags. It does a if check to make sure the tag contains -rc:

     ${{ 
        startsWith(github.ref_name, 'apache-arrow-') && 
        contains(github.ref_name, '-rc') && 
        !inputs.odbc_release_step 
      }}

Since having a tags: section will impact all jobs in the cpp_extra.yml file, I used the if statement approach to the job odbc-msvc-upload-dll.

@justing-bq
justing-bq force-pushed the gh-49537-ci-sign-win-odbc branch from 5106e27 to ad511e0 Compare May 29, 2026 19:20
@amoeba

amoeba commented May 29, 2026

Copy link
Copy Markdown
Member

@raulcd or @kou would either if you like to review again before we merge? I'll do one more review next week but I want to give you both a chance to review if you'd like.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread dev/release/07-flightsqlodbc-upload.sh
Comment thread .github/workflows/cpp_extra.yml Outdated
Comment thread .github/workflows/package_odbc.yml Outdated
Comment thread .github/workflows/package_odbc.yml Outdated
Comment thread dev/release/07-flightsqlodbc-upload.sh Outdated
Comment thread .github/workflows/package_odbc.yml
Comment thread .github/workflows/package_odbc.yml
odbc-msvc-upload-msi:
name: ODBC Windows Build & Upload Unsigned MSI
runs-on: windows-2022
if: inputs.odbc_release_step

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We may be able to remove all inputs for workflow_dispatch and just check whether this workflow is triggered by workflow_dispatch or not.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We can raise an issue for this.

@justing-bq Please raise one GitHub issue for:

  • removing inputs for workflow_dispatch in package_odbc.yml.
  • Then if: github.event_name == 'workflow_dispatch' can be used.
  • The dev/release/07-flightsqlodbc-upload.sh and release documentation needs to be updated as well.

cc @amoeba

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels May 31, 2026
Comment thread .github/workflows/package_odbc.yml Outdated

@alinaliBQ alinaliBQ left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks folks for the reviews. @justing-bq please have a look when you get a chance

Comment thread .github/workflows/package_odbc.yml
Comment thread .github/workflows/package_odbc.yml Outdated
Comment thread .github/workflows/package_odbc.yml
odbc-msvc-upload-msi:
name: ODBC Windows Build & Upload Unsigned MSI
runs-on: windows-2022
if: inputs.odbc_release_step

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We can raise an issue for this.

@justing-bq Please raise one GitHub issue for:

  • removing inputs for workflow_dispatch in package_odbc.yml.
  • Then if: github.event_name == 'workflow_dispatch' can be used.
  • The dev/release/07-flightsqlodbc-upload.sh and release documentation needs to be updated as well.

cc @amoeba

@justing-bq
justing-bq force-pushed the gh-49537-ci-sign-win-odbc branch from ad511e0 to 8b552fe Compare June 4, 2026 18:11
@github-actions github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jun 4, 2026
@justing-bq
justing-bq force-pushed the gh-49537-ci-sign-win-odbc branch from 8b552fe to b4d7f51 Compare June 4, 2026 18:26
@amoeba

amoeba commented Jun 8, 2026

Copy link
Copy Markdown
Member

Hi @justing-bq, I reviewed the recent PR feedback and I think every comment has either been addressed or had an issue created for it. Is that right? Can you fix the merge conflicts and then comment that the PR is ready for a final review?

@justing-bq
justing-bq force-pushed the gh-49537-ci-sign-win-odbc branch from b4d7f51 to 6ba6283 Compare June 8, 2026 17:20
@justing-bq

Copy link
Copy Markdown
Contributor

Hi @justing-bq, I reviewed the recent PR feedback and I think every comment has either been addressed or had an issue created for it. Is that right? Can you fix the merge conflicts and then comment that the PR is ready for a final review?

Hi @amoeba. Yes that is correct. I fixed the merge conflict and this PR is ready for a final review.

@amoeba

amoeba commented Jun 8, 2026

Copy link
Copy Markdown
Member

I think we can merge this once we get one more PR approval. cc @kou @raulcd

@kou kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1

Comment thread .github/workflows/package_odbc.yml Outdated
@github-actions github-actions Bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Jun 9, 2026
* Add draft code for CI A and CI B

Attempt workflow dispatch

Only ODBC Windows original workflow should run.
Later need to add `github.event_name != 'workflow_dispatch' ||` to all existing workflows after uncomment

Use `GITHUB_REF_NAME` directly via push

Add `workflow_dispatch` definitions

Add `ODBC Windows Upload DLL`

Use common ODBC Windows environment variables

Use ODBC as composite action

Create cpp_odbc.yml

Initial draft

temp disable test step

Temp disable non-ODBC Windows workflows

* Clean Up Code

* Remove comments

* Fix Installer path for MSI

Trigger CI

Fix issue with `secrets.GITHUB_TOKEN`

Change `odbc_msi_upload` to boolean input

Change `odbc-msvc-upload-dll` to be triggered via rc tag and can be invoked manually

Work on Bryce's code review comments

^ Conflicts:
^	.github/workflows/cpp_extra.yml
@justing-bq
justing-bq force-pushed the gh-49537-ci-sign-win-odbc branch from 6ba6283 to f491475 Compare June 9, 2026 17:42
@justing-bq

Copy link
Copy Markdown
Contributor

@amoeba @kou
Comment addressed and PR is ready for another review.

@amoeba
amoeba merged commit 9265a71 into apache:main Jun 9, 2026
26 of 27 checks passed
@amoeba amoeba removed the awaiting merge Awaiting merge label Jun 9, 2026
@github-actions github-actions Bot added the awaiting committer review Awaiting committer review label Jun 9, 2026
@amoeba

amoeba commented Jun 9, 2026

Copy link
Copy Markdown
Member

Merged. Thanks for the work on this @alinaliBQ and @justing-bq and thanks for the reviews @raulcd and @kou.

@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 9265a71.

There weren't enough matching historic benchmark results to make a call on whether there were regressions.

The full Conbench report has more details.

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

Labels

awaiting committer review Awaiting committer review CI: Extra: C++ Run extra C++ CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants