Skip to content
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

INTMDB-481: Add support for identity provider display name import #924

Merged
merged 4 commits into from
Nov 15, 2022

Conversation

martinstibbe
Copy link
Contributor

@martinstibbe martinstibbe commented Nov 15, 2022

Description

Add support for identity provider display name import

Link to any related issue(s):
HELP-39564

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the contribution guidelines
  • I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code

Further comments

@martinstibbe martinstibbe requested a review from a team as a code owner November 15, 2022 20:26
@Zuhairahmed Zuhairahmed self-requested a review November 15, 2022 21:22
Copy link
Contributor

@Zuhairahmed Zuhairahmed left a comment

Choose a reason for hiding this comment

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

This looks good, but can we also add 1 more commit to update our terraform registry docs too: https://registry.terraform.io/providers/mongodb/mongodbatlas/latest/docs/resources/federated_settings_org_configs

Copy link
Contributor

@Zuhairahmed Zuhairahmed left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for quick fix on this one @martinstibbe!

@@ -34,6 +35,8 @@ data "mongodbatlas_federated_settings_org_configs" "org_configs_ds" {
* `federation_settings_id` - (Required) Unique 24-hexadecimal digit string that identifies the federated authentication configuration.
* `org_id` - (Required) Unique 24-hexadecimal digit string that identifies the organization that contains your projects.
* `domain_allow_list` - List that contains the approved domains from which organization users can log in.
* `post_auth_role_grants` - (Optional) List that contains the default [roles](https://www.mongodb.com/docs/atlas/reference/user-roles/#std-label-organization-roles) granted to users who authenticate through the IdP in a connected organization. If you provide a postAuthRoleGrants field in the request, the array that you provide replaces the current postAuthRoleGrants.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not blocking but the 2nd half of this doesn't make a great deal of sense to a Terraform user, i.e. "If you provide a postAuthRoleGrants field in the request, the array that you provide replaces the current postAuthRoleGrants."

Copy link
Contributor

Choose a reason for hiding this comment

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

we can update with "List that contains the default roles granted to users who authenticate through the IdP in a connected organization. If you provide a postAuthRoleGrants field in the request, the array that you provide replaces the current postAuthRoleGrants."

source - https://www.mongodb.com/docs/atlas/reference/api/org-mapping-update-one/#request-body-parameters:~:text=postAuthRoleGrants,current%20postAuthRoleGrants.

Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

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

LGTM, see comment that it's blocking but would make sense to edit.

@martinstibbe martinstibbe merged commit a2ba298 into release-staging-v1.6.1 Nov 15, 2022
@martinstibbe martinstibbe deleted the INTMDB-481 branch November 15, 2022 23:55
evertsd pushed a commit that referenced this pull request Nov 17, 2022
* Add support for identity provider display name import

* Update example for mongodbatlas_federated_settings_org_config

* Update documentation for mongodbatlas_federated_settings_org_configs

* Remove confusing verbiage
Zuhairahmed added a commit that referenced this pull request Dec 1, 2022
* INTMDB-438 ForceNew on cluster name change (#902)

* Added ForceNew for name on Cluster

* Updated documentation to include information on name changes for cluster

* INTMDB-481: Add support for identity provider display name import (#924)

* Add support for identity provider display name import

* Update example for mongodbatlas_federated_settings_org_config

* Update documentation for mongodbatlas_federated_settings_org_configs

* Remove confusing verbiage

* INTMDB-498: Updated Prometheus Example (#942)

* Serverless Endpoint Service Doc Bug (#930)

* Update CHANGELOG.md

* Chore(deps): Bump github.com/gruntwork-io/terratest (#936)

Bumps [github.com/gruntwork-io/terratest](https://github.com/gruntwork-io/terratest) from 0.41.0 to 0.41.3.
- [Release notes](https://github.com/gruntwork-io/terratest/releases)
- [Commits](gruntwork-io/terratest@v0.41.0...v0.41.3)

---
updated-dependencies:
- dependency-name: github.com/gruntwork-io/terratest
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add additional context for prometheus example

* Update Readme.md

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Zuhair Ahmed <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* INTMDB-484: Skip tests for OPS GENIE and GOV (#937)

* Serverless Endpoint Service Doc Bug (#930)

* Update CHANGELOG.md

* Skip broken OPS_GENIE token and GOV tests in main ACC test run

* make fmt

* Remove comment

Co-authored-by: Zuhair Ahmed <[email protected]>

* INTMDB-438 ForceNew on cluster name change (#929)

* Added ForceNew for name on Cluster

* Updated documentation to include information on name changes for cluster

* INTMDB-434 - Fixed private_endpoint output documentation (#907)

* Fixed documentation to include more accurate information on how to output a private_endpoint by endpoint_id

* Updated cluster docs to have a generic example for getting connection_string from endpoint_service_id, and links to full examples for AWS, AZURE, and GCP

* Updated examples to have a reference to a cluster, optionally, in order to extract the connection strings

* Updated readme to contain information on the optional cluster_name variable

* Removed moved documentation and added example output in comment

* formatting

* formatting -GCP

* Typo fix

* typo fix - azure

* Apply suggestions from code review

Copy updates on example readme

Co-authored-by: Zuhair Ahmed <[email protected]>

* Updated advanced_cluster documentation to have information on getting connection string

Co-authored-by: Zuhair Ahmed <[email protected]>

* INTMDB-493: Serverless Private Endpoint Connection String Example Fix (#940)

* Serverless Endpoint Service Doc Bug (#930)

* Update CHANGELOG.md

* Change connection_strings_private_srv to a list of connections

* Update connection_strings_private_srv to connection_strings_private_endpoint_srv

Co-authored-by: Zuhair Ahmed <[email protected]>

* INTMDB-426: Alert Configuration -- Api Token erroneous changes (#941)

* Serverless Endpoint Service Doc Bug (#930)

* Update CHANGELOG.md

* Ignore API keys after apply

Co-authored-by: Zuhair Ahmed <[email protected]>

* INTMDB-368: Shorten test names that are too long to allow for targeting specific tests (#932)

* Serverless Endpoint Service Doc Bug (#930)

* Update CHANGELOG.md

* Split tests in segments to permit retry failed test areas in less time

* Change Regex

* skip TestAccConfigRSThirdPartyIntegration_basic

* skip TestAccNetworkRSPrivateEndpointRegionalMode_basic

* Skip Gov tests during regular test run

* Skip OPS_GENIE test

* Change skip test method

* Split out cluster RS and DS

* Single thread private link ADL tests that conflict during parallel execution

* Split out Clusters

* Lower max number of tests to run at once

* Lower concurrent jobs due to cluster limits

* Chain job deps to enforce order

* Combine Clusters back into one workflow

* Skip TestAccClusterRSAdvancedCluster_Paused

* go fmt

Co-authored-by: Zuhair Ahmed <[email protected]>

* INTMDB-463 Cluster container_id documentation (#931)

* container_id documentation updated to reflect that it's a computed field

* Updated documentation around network_peering

* Moved container_id to attribute reference

* Fixed documentation reference

* INTMDB-478: Auto-Generate Changelog (#944)

* Serverless Endpoint Service Doc Bug (#930)

* Update CHANGELOG.md

* Chore(deps): Bump github.com/gruntwork-io/terratest (#936)

Bumps [github.com/gruntwork-io/terratest](https://github.com/gruntwork-io/terratest) from 0.41.0 to 0.41.3.
- [Release notes](https://github.com/gruntwork-io/terratest/releases)
- [Commits](gruntwork-io/terratest@v0.41.0...v0.41.3)

---
updated-dependencies:
- dependency-name: github.com/gruntwork-io/terratest
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Create .github_changelog_generator

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Zuhair Ahmed <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* INTMDB-465 - PrivateLink Endpoint Service & Connection Strings (#943)

* INTMDB-384 Fix PrivateLink test flakiness (#895)

* Using different endpointID values for the endpoint service adl tests

* Removed parallelization with private link endpoint test which had overlap with others

* Reverted changes to the endpointID

* Waiting for Clusters to be idle on pes create

* Formatted and added Deleted as a target status

* Removed unintended changes

* Updated Delete to use TimeoutDelete

* Utilizing error when ClusterListAdvancedRefreshFunc fails

* Updated RefreshFunc to just return PENDING if cluster is not IDLE

* Updated StateChanegConf for endpoints vs. cluster status

* Fixed Pending/Target status' for PrivateEndpointServiceLinkDelete

* Added INITIATING back to list of Pending statuses on delete

* Added serverless instance list refresh func & utlizing it on serverless endpoint service

* Removed unintentional changes to tests and using a proper error message string

* Fixed error message wording to reference proper resource

* Removed extra quote on serverless instance

* WaitForStateContext inline with conditional

* Added documentation on completion condition for endponit service creation & deletion

* INTMDB-470: Private Endpoint Regional Mode test fixes (#946)

* Added a separate test for the basics of perm that does not require aws access

* Skipping complex test

* Renamed tests to fit more w/refac

* Updated RefreshFunc to just return PENDING if cluster is not IDLE

* Updated StateChanegConf for endpoints vs. cluster status

* Fixed Pending/Target status' for PrivateEndpointServiceLinkDelete

* Added INITIATING back to list of Pending statuses on delete

* Putting WaitForStateContext inline with condition checking if it err

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Dosty Everts <[email protected]>
Co-authored-by: Zuhair Ahmed <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

3 participants