fixes for upstream sync PR #52
Conversation
Code Review Agent Run #fca187Actionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Changelist by BitoThis pull request implements the following key changes.
|
5133ee9 to
a8e250d
Compare
There was a problem hiding this comment.
LGTM.
@indradhanush are you looking into moving the image to quay.io as part of a separate issue? (since we are hitting rate limits with docker - which causes the Teamcity build to fail too)
Yes! Sorry it fell through the gaps for me combined with my access key to quay.io not working (open request with IT portal in progress). I'll look into it today. Might sync with you to for the quay.io access for testing. |
There was a problem hiding this comment.
Code Review Agent Run #ab5f77
Actionable Suggestions - 1
-
connector/keystone/keystone.go - 1
- Domain name case change may break authentication · Line 342-342
Additional Suggestions - 1
-
Makefile - 1
-
Duplicated build flag in go build commands · Line 41-42The `-buildvcs=false` flag is added to prevent Go from embedding VCS information in the binary. This is a good practice for reproducible builds, but it's duplicated in both build commands. Consider extracting this flag to the `LD_FLAGS` variable to avoid duplication.
Code suggestion
@@ -38,8 +38,9 @@ .PHONY: release-binary -release-binary: LD_FLAGS = "-w -X main.version=$(VERSION) -extldflags \"-static\"" +release-binary: BUILD_FLAGS = "-buildvcs=false" +release-binary: LD_FLAGS = "-w -X main.version=$(VERSION) -extldflags \"-static\"" release-binary: ## Build release binaries (used to build a final container image). - @go build -buildvcs=false -o /go/bin/dex -v -ldflags $(LD_FLAGS) $(REPO_PATH)/cmd/dex - @go build -buildvcs=false -o /go/bin/docker-entrypoint -v -ldflags $(LD_FLAGS) $(REPO_PATH)/cmd/docker-entrypoint + @go build $(BUILD_FLAGS) -o /go/bin/dex -v -ldflags $(LD_FLAGS) $(REPO_PATH)/cmd/dex + @go build $(BUILD_FLAGS) -o /go/bin/docker-entrypoint -v -ldflags $(LD_FLAGS) $(REPO_PATH)/cmd/docker-entrypoint
-
Review Details
-
Files reviewed - 3 · Commit Range:
a8e250d..a8e250d- Makefile
- connector/keystone/keystone.go
- connector/keystone/keystone_test.go
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Golangci-lint (Linter) - ✖︎ Failed
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at mithil@platform9.com.
Documentation & Help
AI Code Review powered by
| @@ -340,7 +339,7 @@ func (p *conn) authenticate(ctx context.Context, username, pass string) (string, | |||
|
|
|||
| func (p *conn) getAdminTokenUnscoped(ctx context.Context) (string, error) { | |||
| domain := domainKeystone{ | |||
| Name: "default", | |||
| Name: "Default", | |||
There was a problem hiding this comment.
The domain name is being changed from 'default' to 'Default' which may cause authentication failures if the Keystone service expects the domain name to be lowercase. OpenStack Keystone domains are typically case-sensitive.
Code suggestion
Check the AI-generated fix before applying
| Name: "Default", | |
| Name: "default", |
Code Review Run #ab5f77
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
029c2d0
into
private/shubham/sync-upstream-aug-2025
Fixes: KAAP-856
Overview
What this PR does / why we need it
Special notes for your reviewer
Does this PR introduce a user-facing change?
Summary by Bito
This pull request optimizes the build process by disabling VCS information and removing obsolete Makefile targets. It includes bug fixes for the Keystone connector, particularly addressing domain naming inconsistencies and error handling. The test suite has been enhanced with updated host configurations, improved logging, and stronger validation checks.