refactor: update ubuntu/authd paths to canonical/authd#1332
refactor: update ubuntu/authd paths to canonical/authd#1332nooreldeenmansour wants to merge 2 commits intomainfrom
ubuntu/authd paths to canonical/authd#1332Conversation
a78cb2c to
0741417
Compare
| - https://github.com/ubuntu/authd | ||
| issues: https://github.com/ubuntu/authd/issues | ||
| source-code: https://github.com/ubuntu/authd-oidc-brokers/tree/msentraid | ||
| website: https://github.com/canonical/authd |
There was a problem hiding this comment.
Should we update the website in msentraid/google snaps to be consistent with oidc/snapcraft.yaml
There was a problem hiding this comment.
Pull request overview
Refactors repository references from ubuntu/authd to canonical/authd across the OIDC brokers module and snap packaging metadata, while preserving DBus interface naming.
Changes:
- Update Go imports and module dependencies to
github.com/canonical/authd(notably the sharedlogpackage). - Adjust snapcraft builds to compile the brokers from the repo root (to accommodate a temporary Go module replace).
- Update README/codecov links and snap metadata URLs to the Canonical GitHub namespace.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| snap/variants/oidc/snapcraft.yaml | Build brokers from repo root; add temporary build workaround comments. |
| snap/variants/msentraid/snapcraft.yaml | Update metadata URLs; build brokers from repo root; update ldflags import path. |
| snap/variants/google/snapcraft.yaml | Update metadata URLs; build brokers from repo root; update ldflags import path. |
| authd-oidc-brokers/internal/testutils/provider.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/internal/providers/msentraid/msentraid_test.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/internal/providers/msentraid/msentraid.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau_c.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/internal/providers/msentraid/himmelblau/himmelblau.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/internal/dbusservice/methods.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/internal/daemon/daemon.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/internal/consts/consts.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/internal/broker/encrypt.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/internal/broker/broker_test.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/internal/broker/broker.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/go.sum | Drop ubuntu/authd sums; bump go-systemd/v22 sums. |
| authd-oidc-brokers/go.mod | Replace module dependency from ubuntu/authd to canonical/authd and add temporary replace to ../. |
| authd-oidc-brokers/cmd/authd-oidc/main.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/cmd/authd-oidc/daemon/daemon.go | Switch log import to github.com/canonical/authd/log. |
| authd-oidc-brokers/cmd/authd-oidc/daemon/config.go | Switch log import to github.com/canonical/authd/log. |
| README.md | Update Codecov badge URLs to canonical/authd. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Temporary until a release tag exists whose go.mod declares module github.com/canonical/authd. | ||
| // TODO: Remove replace after that release and update the required version. | ||
| replace github.com/canonical/authd => ../ |
There was a problem hiding this comment.
The build fails if this line was removed at the moment.
snap/variants/google/snapcraft.yaml
Outdated
| go mod vendor | ||
| VERSION=$(craftctl get version) | ||
| export GOFLAGS="-ldflags=-X=github.com/ubuntu/authd-oidc-brokers/internal/consts.Version=${VERSION}" | ||
| export GOFLAGS="-ldflags=-X=github.com/canonical/authd/authd-oidc-brokers/internal/consts.Version=${VERSION}" |
There was a problem hiding this comment.
Out of scope of this change, should we address it in the same PR?
There was a problem hiding this comment.
we should investigate if this is an actual issue and the impact it has
There was a problem hiding this comment.
I've tried investigating it, my findings are the impact is, msentraid broker would be affected because, when the release build tag is not set, the source directory is embedded as an RPATH in the binary
A quick way to test this locally
$ cd authd-oidc-brokers
$ go build -tags=withmsentraid -o /tmp/binary ./cmd/authd-oidc
$ go build -tags=withmsentraid,release -o /tmp/binary-release ./cmd/authd-oidc
$ readelf -d /tmp/binary | grep RUNPATH
0x000000000000001d (RUNPATH) Library runpath: [/home/nooreldeenmansour/.../himmelblau]
$ readelf -d /tmp/binary-release | grep RUNPATH
(no output)
In the actual snap build, the embedded path would be the snapcraft container's internal build directory.
I've amended the commit by removing GOFLAGS from build-environment and passing -tags directly on the go build command (this fixes the overwriting issue reported by copilot). Let me know if you wish to undo this change, It's unclear to me whether addressing this is important or not.
There was a problem hiding this comment.
thanks for the investigation. I agree that we should fix that and I'm fine with the solution and piggybacking the fix on this unrelated PR, but please do it in a separate commit.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1332 +/- ##
==========================================
- Coverage 86.41% 80.08% -6.34%
==========================================
Files 99 20 -79
Lines 6690 984 -5706
Branches 111 0 -111
==========================================
- Hits 5781 788 -4993
+ Misses 853 196 -657
+ Partials 56 0 -56 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
0741417 to
01128d6
Compare
snap/variants/google/snapcraft.yaml
Outdated
| source-code: https://github.com/ubuntu/authd-oidc-brokers/tree/google | ||
| website: https://github.com/canonical/authd | ||
| issues: https://github.com/canonical/authd/issues | ||
| source-code: https://github.com/canonical/authd |
There was a problem hiding this comment.
| source-code: https://github.com/canonical/authd | |
| source-code: https://github.com/canonical/authd/tree/main/authd-oidc-brokers |
There was a problem hiding this comment.
same in the other two snapcraft.yaml
01128d6 to
e3a7c5f
Compare
The `export GOFLAGS="-ldflags=..."` in override-build was overwriting the `GOFLAGS: "-tags=release"` set in build-environment, causing the release tag to be silently dropped during compilation. This tag controls whether the build-time source directory is embedded as RPATH in the binary for msentraid broker Moved `-tags=release` directly to each variant's go build command line and passed `-ldflags` as a flag instead GOFLAGS export. This ensures the tag is always applied and makes all flags explicit and visible.
e3a7c5f to
446f564
Compare
Updated all references of
ubuntu/authdtocanonical/authd, with exception to DBus paths, and Launchpad PPA urls.UDENG-9438