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

Removes embedded Okta Groups #2

Merged
merged 7 commits into from
Nov 3, 2020
Merged

Removes embedded Okta Groups #2

merged 7 commits into from
Nov 3, 2020

Conversation

fernandogoncalves-me
Copy link
Contributor

@fernandogoncalves-me fernandogoncalves-me commented Nov 2, 2020

The current value for local.okta_groups is fixed and a mix of AWS and Github groups.

This change proposes separating AWS groups into a dedicate list and supporting custom groups in order to accommodate different needs. However, it still offers a default group AWSPlatformAdmins to avoid having to always pass a group name.

@shoekstra
Copy link
Member

shoekstra commented Nov 2, 2020

I'd like to put this on hold until support for SSO permission sets has been merged until the AWS provider.

While it's just okta groups it doesn't make sense to be part of this module; if it's Okta groups together with permission sets then it makes sense.

edit: set this to draft so we can discuss before this is merged

@shoekstra shoekstra marked this pull request as draft November 2, 2020 19:14
@marwinbaumannsbp
Copy link
Contributor

On the one hand I agree with you that its better to add support for Okta groups i.c.w. SSO permission sets so the full solution is implemented, but on the other hand not merging this makes the migration of the Geldmaat inception harder. If we are not going to merge this, are we then going to document that the creation of Okta groups is manual for now and remove this from the state? Maybe we should have a short call about this today! :-)

@fernandogoncalves-me fernandogoncalves-me changed the title Adds support for custom AWS Okta Groups Removes embedded Okta Groups Nov 3, 2020
@fernandogoncalves-me
Copy link
Contributor Author

We have decided to remove the Okta Groups from the module and manage them separately until we can implement the full solution including SSO permission set assignments.

@fernandogoncalves-me fernandogoncalves-me marked this pull request as ready for review November 3, 2020 09:50
variables.tf Outdated Show resolved Hide resolved
Copy link
Member

@shoekstra shoekstra left a comment

Choose a reason for hiding this comment

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

Minor fix required 🙂

fernandogoncalves-me and others added 2 commits November 3, 2020 11:13
Co-authored-by: Stephen Hoekstra <shoekstra@schubergphilis.com>
@fernandogoncalves-me fernandogoncalves-me merged commit 035c8b2 into master Nov 3, 2020
@fernandogoncalves-me fernandogoncalves-me deleted the okta_groups branch November 3, 2020 10:31
@burck1
Copy link

burck1 commented Nov 3, 2020

Hi @fgoncalves-io, @shoekstra, @marwinbaumannsbp.

In the interest of getting the AWS SSO resources merged, please go give a thumbs up to 15808

We've completed most of the work for supporting the AWS SSO and AWS SSO Identity Store resources and datasources in Terraform. The 15322 [WIP] PR encompasses all of that work. But, the contribution guide for the repo recommends submitting small pull requests with the minimum required resources, so we've submitted 15808 as our initial PR with just data.aws_sso_instance, data.aws_sso_permission_set, and aws_sso_permission_set. Once that's merged, we will submit PRs for all of the other resources and data sources since they depend on that initial PR.

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.

4 participants