Skip to content

CLOUDP-314903 [OIDC] CRD Config Propagation to Automation Config #60

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

Merged
merged 50 commits into from
Jun 2, 2025

Conversation

MaciejKaras
Copy link
Collaborator

@MaciejKaras MaciejKaras commented Apr 29, 2025

Summary

Core Functionality Enhancements:

  • Added a new authentication mechanism, MongoDB-OIDC, to the list of supported mechanisms in the authentication_mechanism.go file.
  • Introduced the OIDCProviderConfigs field in the AutomationConfig struct and implemented logic to merge and apply OIDC configurations into the deployment in the automation_config.go file.
  • Removed default value for groupClaim because the value groups can result in hard to debug misconfiguration.

API and Configuration Updates:

  • Added the IsOIDCEnabled() method in the Security struct and AuthResource interface to check if OIDC is enabled.
  • Updated the Options struct in the authentication.go file to include OIDCProviderConfigs.

Test Coverage:

  • Added comprehensive test cases for OIDC provider configurations in automation_config_test.go, including scenarios for merging, clearing, and modifying configurations.
  • Updated the TestAutomationConfigEquality test to include OIDC provider configurations.

JSON Configuration Example:

  • Updated the automation_config.json test data file to include sample OIDC provider configurations for testing purposes.

Proof of Work

Checklist

  • Have you linked a jira ticket and/or is the ticket in the title?
  • Have you checked whether your jira ticket required DOCSP changes?
  • Have you checked for release_note changes?

Reminder (Please remove this when merging)

  • Please try to Approve or Reject Changes the PR, keep PRs in review as short as possible
  • Our Short Guide for PRs: Link
  • Remember the following Communication Standards - use comment prefixes for clarity:
    • blocking: Must be addressed before approval.
    • follow-up: Can be addressed in a later PR or ticket.
    • q: Clarifying question.
    • nit: Non-blocking suggestions.
    • note: Side-note, non-actionable. Example: Praise
    • --> no prefix is considered a question

@MaciejKaras MaciejKaras force-pushed the feature/mk-oidc-crd-propagation branch from cad403d to 0ce0874 Compare April 30, 2025 07:46
@anandsyncs anandsyncs self-requested a review May 27, 2025 14:38
Copy link
Member

@mircea-cosbuc mircea-cosbuc left a comment

Choose a reason for hiding this comment

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

LGTM, I think my comments just require clarification rather than changes.

Copy link
Contributor

@lsierant lsierant left a comment

Choose a reason for hiding this comment

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

I've left few comments: most are minor, but I've put a blocking one regarding MergoDelete.
I also miss a bit a proper e2e test verifying the automation config changes in a real world scenario.

@MaciejKaras MaciejKaras requested a review from lsierant May 29, 2025 13:52
@anandsyncs
Copy link
Contributor

evergreen retry

# Conflicts:
#	api/v1/mdb/mongodb_types.go
@MaciejKaras MaciejKaras force-pushed the feature/mk-oidc-crd-propagation branch from 1f504cf to 633c456 Compare June 2, 2025 08:23
Copy link
Contributor

@lsierant lsierant left a comment

Choose a reason for hiding this comment

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

LGTM! Great work 👏

@MaciejKaras MaciejKaras merged commit f4465ab into master Jun 2, 2025
33 of 35 checks passed
@MaciejKaras MaciejKaras deleted the feature/mk-oidc-crd-propagation branch June 2, 2025 17:03
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.

7 participants