Skip to content

Conversation

@mateoHernandez123
Copy link
Contributor

@mateoHernandez123 mateoHernandez123 commented Dec 4, 2025

Pull Request

To help us get this pull request reviewed and merged quickly, please be sure to include the following items:

  • Tests (if applicable)
  • Documentation (if applicable)
  • Changelog entry
  • A full explanation here in the PR description of the work done

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Tests
  • Other

Backward Compatibility

Is this change backward compatible with the most recently released version? Does it introduce changes which might change the user experience in any way? Does it alter the Connector in any way?

  • Yes (backward compatible)
  • No (breaking changes)

Useful links:

  • [https://github.com/ConductorOne/baton-sdk/wiki/Coding-Guidelines](Baton SDK coding guidelines)
  • [https://github.com/ConductorOne/baton/blob/main/CONTRIBUTING.md](New contributor guide)

Issue Linking

What's new?

Summary by CodeRabbit

  • Chores

    • Updated Go runtime to 1.25.2 and refreshed multiple dependencies (SDK, testing, system libs) for compatibility and performance.
  • Improvements

    • Connector display name standardized to "MongoDB Atlas".
    • Several configuration fields now have explicit default values to simplify setup.
  • Bug Fixes

    • Configuration string handling enhanced to accept byte-encoded values without errors.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 4, 2025

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (3)
  • .github/workflows/release.yaml is excluded by none and included by none
  • baton_capabilities.json is excluded by none and included by none
  • config_schema.json is excluded by none and included by none

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Go toolchain and multiple module dependencies updated; GetString in generated config now accepts both string and []byte; several config fields' defaults changed and connector display name updated.

Changes

Cohort / File(s) Summary
Module / Dependency manifest
go.mod
Go version updated to 1.25.2. Public deps updated (e.g., github.com/conductorone/baton-sdk bumped to v0.6.24, github.com/stretchr/testify to v1.11.1). Indirect deps adjusted: add Masterminds/semver/v3, ebitengine/purego; upgrade various indirects (e.g., maypok86/otter v1→v2, shirou/gopsutil v3→v4); some indirects removed.
Generated config logic
pkg/config/conf.gen.go
Receiver formatting fixed (cosmetic). GetString extended to return string for string or []byte inputs (converts []byte to string); panics only if neither type matches.
Connector configuration
pkg/config/config.go
Several boolean fields changed from WithRequired(false) to WithDefaultValue(...) (CreateInviteKeyField, EnableSyncDatabases, EnableMongoDriver, DeleteDatabaseUserWithReadOnly). Connector display name updated from "MongodbAtlas" to "MongoDB Atlas"; no signature changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 I hopped through modules, tidy and spry,
Strings and bytes now share a pie.
Defaults snugged in, names polished bright,
A rabbit's small patch—clean code delight. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title references a Jira ticket BB-1822 and mentions 'Containerize Connector - MongoDB', but the actual changes are dependency updates and configuration adjustments unrelated to containerization. Update the title to reflect the actual changes, such as 'Update baton-sdk and dependencies, configure MongoDB Atlas display name and field defaults' or similar.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@laurenleach
Copy link
Contributor

@mateoHernandez123 @btipling we tried to containerize this previously. It did not work for the database stuff, it needs sock5, chris did some work to support this but I think we need to update the connector to use this

the behavior it had when we containerized: syncing successfully but dropping any database resources

@mateoHernandez123
Copy link
Contributor Author

mateoHernandez123 commented Dec 9, 2025

@mateoHernandez123 @btipling we tried to containerize this previously. It did not work for the database stuff, it needs sock5, chris did some work to support this but I think we need to update the connector to use this

the behavior it had when we containerized: syncing successfully but dropping any database resources

I just updated the SDK version in this same PR. Can you confirm if it works? @laurenleach

@mateoHernandez123 mateoHernandez123 changed the title [BB-1822] - Containerize Connector [BB-1822] - Containerize Connector - MongoDB Dec 13, 2025
@luisina-santos luisina-santos force-pushed the mateoHernandez123/containerize-connector branch from b3e2ab1 to 330b619 Compare January 22, 2026 13:51
@luisina-santos luisina-santos requested a review from a team January 22, 2026 13:51
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pkg/config/config.go (1)

22-47: Defaults now enable invite creation and DB sync—confirm this breaking change is intended.

Previously omitted flags would default to false; now they default to true, which can create users/invites and sync databases unexpectedly. If this is intended, please add a migration note and update docs; if not, consider reverting.

💡 Possible rollback to preserve prior defaults
 var CreateInviteKeyField = field.BoolField(
 	"create-invite-key",
 	field.WithDisplayName("Create Invite"),
 	field.WithDescription("If enabled, Baton will create invites for users that do not have an account in MongoDB Atlas when provisioning."),
-	field.WithDefaultValue(true),
+	field.WithRequired(false),
 )
 ...
 var EnableSyncDatabases = field.BoolField(
 	"enable-sync-databases",
 	field.WithDisplayName("Sync Databases"),
 	field.WithDescription("If enabled, Baton will sync database users and roles."),
-	field.WithDefaultValue(true),
+	field.WithRequired(false),
 )
🧹 Nitpick comments (1)
pkg/config/conf.gen.go (1)

44-55: Add a small test for the new []byte path.

This locks in the new behavior and prevents regressions in config loading.

@luisina-santos luisina-santos marked this pull request as draft January 26, 2026 19:29
@luisina-santos luisina-santos self-assigned this Jan 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants