okta / terraform-provider-okta

A Terraform provider to manage Okta resources, enabling infrastructure-as-code provisioning and management of users, groups, applications, and other Okta objects.
https://registry.terraform.io/providers/okta/okta
Mozilla Public License 2.0
259 stars 209 forks source link

Version v5.0.0 wish list #1183

Open monde opened 2 years ago

monde commented 2 years ago

Using this issue to gather suggestions for v5.0.0 of the provider which will occur sometime this year. Anything that is incompatible with v3.X.X / v4.X.X or incongruent with TF design principles https://developer.hashicorp.com/terraform/plugin/best-practices/hashicorp-provider-design-principles is on the table.

BalaGanaparthi commented 2 years ago

Resource definitions must be conducive to dynamic blocks.

Example : The current resource resource okta_policy_mfa has individual authenticators defined like

Note : Picking on okta_policy_mfa just to highlight the concept, but the same thought process is applicable for new resources or adapting the principles to the existing resource (or datasource) definitions .. where ever possible

resource "okta_policy_mfa" "MFAPolicy_master" {
  name        = "Service / Master Accounts"
  status      = "ACTIVE"
  description = "MFA Policy for Master Accounts"
  priority    = 1

  okta_verify = {
    enroll = "NOT_ALLOWED"
  }
  phone_number = {
    enroll = "NOT_ALLOWED"
  }
  webauthn = {
    enroll = "NOT_ALLOWED"
  }
  fido_u2f = {
    enroll = "NOT_ALLOWED"
  }
  okta_question = {
    enroll = "REQUIRED"
  }
  yubikey_token = {
    enroll = "NOT_ALLOWED"
  }
}

This resource definition can be formatted as

resource "okta_policy_mfa" "MFAPolicy_master" {
  name        = "Service / Master Accounts"
  status      = "ACTIVE"
  description = "MFA Policy for Master Accounts"
  priority    = 1

   authenticator  {
      type = okta_verify
      enroll = "NOT_ALLOWED"
   }
   authenticator  {
      type = phone_number
      enroll = "NOT_ALLOWED"
   }
   authenticator  {
      type = webauthn
      enroll = "NOT_ALLOWED"
   }
   authenticator  {
      type = fido_u2f
      enroll = "NOT_ALLOWED"
   }
   authenticator  {
      type = okta_question
      enroll = "REQUIRED"
   }
   authenticator {
      type = yubikey_token
      enroll = "NOT_ALLOWED"
   }
}

With the new resource definition format the resource block can be made very concise when multiple resource with different internal configurations are required to be managed.. where the script and data separation can be achieved... like the following

Data in JSON format

[
    {
       "name"          : "MFA Policy-1,
       "status"         : "ACTIVE",
       "description" : "This is MFA Policy One",
       "is_oie"          : True,
       "authenticators" : [
              {
                  "type" : "okta_verify"
                  "enroll" : "NOT_ALLOWED"
              },
              {
                  "type" : "phone_number"
                  "enroll" : "NOT_ALLOWED"
              },
              {
                  "type" : "webauthn"
                  "enroll" : "NOT_ALLOWED"
              },
              {
                  "type" : "okta_question"
                  "enroll" : "REQUIRED"
              },
       ]
    },
    {
       "name"          : "MFA Policy-2,
       "status"         : "ACTIVE",
       "description" : "This is MFA Policy Two",
       "is_oie"          : True,
       "authenticators" : [
              {
                  "type" : "okta_verify"
                  "enroll" : "REQUIRED"
              },
              {
                  "type" : "phone_number"
                  "enroll" : "NOT_ALLOWED"
              },
              {
                  "type" : "webauthn"
                  "enroll" : "NOT_ALLOWED"
              },
              {
                  "type" : "okta_question"
                  "enroll" : "DISABLED"
              },
       ]
    },
]

The script can be made concise and modularized for the data to be passed from external sources hence achieving good data and code separation

resource "okta_policy_mfa" "mfa_policies" {
   for_each = var.mfa_enroll_policy_list

   name          = each.value.name
   status         = each.value.status
   description = each.value.description
   is_oie          = each.value.is_oie

   dynamic "authenticator" {
      for_each = each.value.authenticators
         content {
            type = each.value.type
            enroll = each.value.enroll
         }
   }
}

The result : Can achieve separation of concerns by isolating the scripts from data.. where each can be managed relevant personnel. Data can evolve (with format conformance with the script) and no need to repeat the resource blocks within the script when multiple resources of the same types must be managed.

noinarisak commented 2 years ago

Documentation

Example: We need a Guides section that would allow new users to understand the following topics: How get started configuring the Provider, Migration instructions on the deprecated resource or data sources, etc.

Example: More consistency within each resources and data sources around examples or best practices.

BalaGanaparthi commented 2 years ago

Documentation

Every resource (and datasource) with a section of Required Okta OAuth Scopes that provides all required scopes to successfully execute the resource (or datasource) when OKTA_API_PRIVATE_KEY is used for credentials.

monde commented 2 years ago

A mechanism to partition feature behavior based on what kind of org the provider is pointed at and what can of feature flags are enabled on that org. I believe there are plans to expose a public API endpoint that will surface the feature flags/capabilities of an org that will allow this.

Also, something similar in the ACC tests. The ACC suite takes about 20 minutes to complete and I have to run it against four or more test orgs that have different capabilities to find truely failing ACC tests.

delize commented 2 years ago

Documentation Improvement


Personally, I would like to see the documentation include what the expected data types are for each argument resource.

An example of this is for the Sign On Policy.

The following arguments are supported:

[name](https://registry.terraform.io/providers/okta/okta/latest/docs/resources/policy_signon#name) - (Required) Policy Name.

[description](https://registry.terraform.io/providers/okta/okta/latest/docs/resources/policy_signon#description) - (Optional) Policy Description.

[priority](https://registry.terraform.io/providers/okta/okta/latest/docs/resources/policy_signon#priority) - (Optional) Priority of the policy.

[status](https://registry.terraform.io/providers/okta/okta/latest/docs/resources/policy_signon#status) - (Optional) Policy Status: "ACTIVE" or "INACTIVE".

[groups_included](https://registry.terraform.io/providers/okta/okta/latest/docs/resources/policy_signon#groups_included) - List of Group IDs to Include.

For each of these, what is the expected value data type outlined per line.

Some might be obvious (name, and description are usually strings), however others like priority are less obvious - Is this a number or a string? It isn't clear on first glance.

This is helpful when setting up "default values" in variables for the expected resource from scratch, rather than importing existing values first.

I also might be overlooking this and it may already be explained somewhere that I am just not looking at.

monde commented 2 years ago

https://github.com/okta/terraform-provider-okta/issues/999

tgoodsell-tempus commented 2 years ago

Lots of deprecated resources already trying to deal with this, however more strict focus on following the Terraform best practices of having the resources not mix and match various API objects: https://www.terraform.io/plugin/hashicorp-provider-design-principles#resources-should-represent-a-single-api-object

For example, the apps, groups, and users APIs should avoid calling both the "root" CRUD APIs as well as the "sub-apis" such as users and groups.

ymylei commented 2 years ago

Sign On Policy

Adding onto this, may be easier to support this request overall if time is spent in re-doing the docs and generating using tfplugindocs. See https://www.terraform.io/registry/providers/docs#generating-documentation

ktham commented 2 years ago

Remove artificial input validation in the provider and allow the API responses dictate correct configuration [...] If the Okta API changes then any artificial input validation in the provider's code can drift

As long as the input validation still happens at plan-time (and not apply-time), I am ok with that. I want to minimize situations where I merge in a PR that generated a valid TF plan, only to have it fail during apply-time.

monde commented 2 years ago

Remove deprecated resources.

BalaGanaparthi commented 2 years ago

Datasources

Use-case this feature will solve

When a customer with existing Okta implementation without terraform would like manage all the existing configurations via terraform going forward

Feature requested

List Data Source : Each Resource have a corresponding list Data Source, where the IDs of all the entities/objects configured in the tenant of that Resource Type are fetched

Full-Entity-Data Data Source : Each Resource have a corresponding list Data Source, where full data of a specific entity/object configured in the tenant of that Resource Type is fetched based on the entity-ID provided as input to the Data Source. The Entity-ID can be fetched from the List Data Source. The output of the Data Source must be directly mapped to the attributes of the corresponding Resource

Slim-Entity-Data Data Source : Each Resource have a corresponding list Data Source, where only attribute data of a specific entity/object configured in the tenant of that Resource Type is fetched based on the entity-ID provided as input to the Data Source. The Entity-ID can be fetched from the List Data Source.

Variation to Slim-Entity-Data Data Source : Accept the list of attribute of the entity/object to be returned.

How the feature will be used

Multiple Provider Configurations

provider "okta" {
  alias = source
  # Configuration options
}

provider "okta" {
  alias = target
  # Configuration options
}

module "groups-source" {
  source          = "./modules/groups-src"
  # Module to get existing group details from source tenant (by using datasources)
  providers = {
    okta = okta.source
  }
} 

module "groups-target" {
  source          = "./modules/groups-target"
  # Module to create groups (using Resource) with the output of groups-source module piped as input to this module
  providers = {
    okta = okta.target
  }
} 
delize commented 2 years ago

Official Okta Modules

Would be nice to have Okta officially come up with dynamic/flexible module uses where it makes sense to package or bundle the resources that work together or are highly functional together (EG: Sign On Policies & Sign On Rule Policies).

While it is great that these are separate resources, they could function together as a module. Having Okta officially outline the "best practice" (cringey word, but at least Okta's internal best practice) would be nice.

monde commented 2 years ago

Thanks @delize . I'm not sure on the follow through, but I do know our product people want to get some sort of reference examples/documention/configurations published for the TF provider. As there are many different ways to use the provider I would think there might be multiple examples.

ruimarinho commented 2 years ago

@monde I think I can provide a script in terms of what I think are onboarding requirements and how those could be translated into this TF provider. E.g. dashboard configuration, authenticator requirements, profile mappings from HR system (which I think are not supported yet?), dynamic group assignments with rules, first SAML app, etc.

monde commented 2 years ago

@ruimarinho awesome, share your config in a gist here if it can be shared in the public else send me your artifacts mike.mondragon@okta.com and I'll be able to share with product/documentation.

@ruimarinho @delize , coincidentally, my recollection was correct, just saw a planning deck today, we are in Q3 at Okta, and providing reference architectures for all of our SDKs and the TF provider is a pilot project this quarter.

talmarco commented 2 years ago

The "Refresh application data" button, which is currently only available from the user interface, would be great to see implemented. Okta is heavily used and integrates with Terraform for automating the creation of applications, groups, and assignments, but there is still a need to manually refresh application data and then re-run Terraform.

monde commented 2 years ago

renamed v5.0.0 . The v4.0.0 release plan is explained here: v4.0.0 release plan #1338

tgoodsell-tempus commented 1 year ago

Something to consider for the v5 milestone, HashiCorp is now recommending we move to terraform-provider-framework now that they've 1.0'd it: https://developer.hashicorp.com/terraform/plugin/which-sdk#are-you-writing-a-new-provider-or-maintaining-an-existing-provider

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days

tgoodsell-tempus commented 1 year ago

Something else for this if considered:

We should cutover the Plugin Protocol used for this provider to V6, see: https://developer.hashicorp.com/terraform/plugin/terraform-plugin-protocol#protocol-version-6

Should open up features and things as we implement (and re-implement) more of the provider in the terraform-plugin-framework.

exitcode0 commented 1 year ago

It might make sense for this one to go in as a part of a V5 release it might require a state file migration? or perhaps a breaking change? https://github.com/okta/terraform-provider-okta/issues/1474

tgoodsell-tempus commented 1 year ago

Based on https://github.com/okta/terraform-provider-okta/issues/1769

I think we should also remove all the client_secret management from okta_app_oauth, this should all be transferred to a separate resource which handles all the nuance and dual value support better.

duytiennguyen-okta commented 1 year ago

Something else for this if considered:

We should cutover the Plugin Protocol used for this provider to V6, see: https://developer.hashicorp.com/terraform/plugin/terraform-plugin-protocol#protocol-version-6

Should open up features and things as we implement (and re-implement) more of the provider in the terraform-plugin-framework.

It is not possible if we still have resource in plugin sdk as v5 protocol is how hashi mux the 2 server together. Have to stay with v5 for now

tgoodsell-tempus commented 1 year ago

Something else for this if considered: We should cutover the Plugin Protocol used for this provider to V6, see: https://developer.hashicorp.com/terraform/plugin/terraform-plugin-protocol#protocol-version-6 Should open up features and things as we implement (and re-implement) more of the provider in the terraform-plugin-framework.

It is not possible if we still have resource in plugin sdk as v5 protocol is how hashi mux the 2 server together. Have to stay with v5 for now

This statement is false, there is a 5to6 mux the same way a 6to5 mux exists, see: https://developer.hashicorp.com/terraform/plugin/mux/translating-protocol-version-5-to-6

Protocol using this is backwards compatible per: https://developer.hashicorp.com/terraform/plugin/terraform-plugin-protocol

duytiennguyen-okta commented 1 year ago

Interesting, I will look into it

monde commented 11 months ago

I'm seeing merit in bringing back input validation. The SDK client okta-sdk-golang v3 that we've slowly been transitioning to already does it by default on enum parameters. We'd only have to bring back validation on data surfaces like min/max for integers and min/max string length, etc. . See #1839

It's a pain to maintain artificial input validation and do timely updates with this provider. It can often block customers using some new ability in the Okta API. However, it seems like everyone would prefer stability that input validation gives on the plan.

lukezilch commented 3 months ago

Would be great if we could get push groups added into the okta provider, one of my single PIA is push groups, some apps have so many that it would be great to add the groups to the “push groups” section via my terraform instead of having to manually assign them