Combine SDKV2 and Framework Plugin Provider #125
Conversation
…-resource Add Server Checkout Settings Resource with new plugin framework
There was a problem hiding this comment.
I think we should follow this structure -
oktapam
- provider
-- framework
--- provider.go
-- sdkv2
--- provider.go
-- factory.go (This is to create a provider Server Factory. This code currently we are duplicating in provider_test.go and main.go)
There was a problem hiding this comment.
Resources and Resource Test can be at the same place for all the providers. So, basically keep them under - oktapam/. I also observed that you havn't created datasource for this.
There was a problem hiding this comment.
Move this file under oktapam/
| "checkout_duration_in_seconds": schema.Int64Attribute{ | ||
| Optional: true, | ||
| Computed: true, | ||
| Description: "The duration in seconds for the checkout. If the checkout is enabled, the duration is the maximum time a user can access the resource before the checkout expires.", |
There was a problem hiding this comment.
We are keeping description in oktapam/constants/descriptions.
| Description: "If provided, only the account identifiers listed are required to perform a checkout to access the resource. This list is only considered if `checkout_required` is set to `true`. Only one of `include_list` and `exclude_list` can be specified in a request since they are mutually exclusive.", | ||
| MarkdownDescription: "If provided, only the account identifiers listed are required to perform a checkout to access the resource. This list is only considered if `checkout_required` is set to `true`. Only one of `include_list` and `exclude_list` can be specified in a request since they are mutually exclusive.", | ||
| }, | ||
| "project": schema.StringAttribute{ |
There was a problem hiding this comment.
This should have force new true. If project is changed in the resource that means deleting the setting for old project and creating new one.
| Description: "The UUID of a Project", | ||
| MarkdownDescription: "The UUID of a Project", | ||
| }, | ||
| "resource_group": schema.StringAttribute{ |
There was a problem hiding this comment.
This should have force new 'true'. If resource group is changed in the resource that means deleting the setting for old project and creating new one.
| plan.Id = types.StringValue(formatServerCheckoutSettingsID(plan.ResourceGroup, plan.Project)) | ||
| diags = resp.State.Set(ctx, plan) | ||
| resp.Diagnostics.Append(diags...) | ||
| if resp.Diagnostics.HasError() { |
There was a problem hiding this comment.
What is the purpose of doing this? Errors are already added in resp.Diagnostics
There was a problem hiding this comment.
I think better to call read after creating the resource so that state will be set based on the returned data. We are following this pattern in other resources too.
| ExcludeList: plan.ExcludeList, | ||
| } | ||
|
|
||
| _, err := r.client.SDKClient.ProjectsAPI.UpdateResourceGroupServerBasedProjectCheckoutSettings(ctx, r.client.Team, plan.ResourceGroup, plan.Project).ResourceCheckoutSettings(*serverCheckoutSettings).Execute() |
There was a problem hiding this comment.
Create a separate file under oktapam/client to call SDK and handle errors there.
| // we manage it as a separate resource since it's lifecycle is somewhat separate from a project. | ||
| // since project password settings are managed as a separate resource with format resourceGroupID/projectID already, | ||
| // we can just append the server_checkout_settings to the end of the ID | ||
| return fmt.Sprintf("%s/%s/server_checkout_settings", resourceGroupID, projectID) |
There was a problem hiding this comment.
You don't need to add server_checkout_settings. For a project and resource, there can be only one server checkout settings.
main.go
Outdated
|
|
||
| // Combine Providers | ||
| // Refer: https://developer.hashicorp.com/terraform/plugin/mux/combining-protocol-version-6-providers | ||
| providers := []func() tfprotov6.ProviderServer{ |
There was a problem hiding this comment.
Move this code out to factory.go as it is used in both provider_test.go and main.go.
`func ProviderServerFactoryV6(ctx context.Context) (func() tfprotov6.ProviderServer, error) {
v5Provider := oktapam.Provider()
v6Provider := fwprovider.New()()
// SDKV2 used for tf plugin development is designed for maintaining tf plugins that are compatible with Plugin
// Protocol version 5. Plugins need to communicate with Terraform CLI, protocol version 5 is supported by CLI version
// 0.12 and later. Protocol version 6 support tf cli version 1.0 or later.
// To start using new TF Plugin Framework(https://developer.hashicorp.com/terraform/plugin/framework) we have two options -
// Option 1: Downgrade new plugin framework server to support protocol version 5
// Option 2: Upgrade old SDKV2 provider server to support protocol version 6
// If we go with Option 1, then will not be able to use some of the newer features like Nested Attributes:
// https://developer.hashicorp.com/terraform/plugin/framework/handling-data/attributes#nested-attribute-types
//Going with option 2, that will require upgrading tf cli version to 1.0+.
// tf5to6server enables translating a protocol version 5 provider server into a protocol version 6 provider server.
upgradedV5Provider, err := tf5to6server.UpgradeServer(
ctx,
v5Provider.GRPCProvider,
)
if err != nil {
return nil, err
}
// Combine Providers
// Refer: https://developer.hashicorp.com/terraform/plugin/mux/combining-protocol-version-6-providers
providers := []func() tfprotov6.ProviderServer{
func() tfprotov6.ProviderServer {
return upgradedV5Provider
},
providerserver.NewProtocol6(v6Provider),
}
muxServer, err := tf6muxserver.NewMuxServer(ctx, providers...)
if err != nil {
return nil, err
}
return muxServer.ProviderServer, nil
}`
There was a problem hiding this comment.
Now, in main.go you just need to do -
muxServer, err := ProviderServerFactoryV6(ctx)
if err != nil {
log.Fatal(err)
}
var serveOpts []tf6server.ServeOpt
if debug {
serveOpts = append(serveOpts, tf6server.WithManagedDebug())
}
err = tf6server.Serve(
"registry.terraform.io/okta.com/pam/oktapam",
muxServer,
serveOpts...,
)
oktapam/provider_test.go
Outdated
| return map[string]func() (tfprotov6.ProviderServer, error){ | ||
| "oktapam": func() (tfprotov6.ProviderServer, error) { | ||
| // upgrade sdk v2 provider to protocol version 6 | ||
| upgradedSdkProvider, err := tf5to6server.UpgradeServer( |
There was a problem hiding this comment.
This code can we moved to oktapam/provider/factory.go as I mentioned in earlier comment.
| ) | ||
|
|
||
| const testAccServerCheckoutSettingsCreateConfigFormat = ` | ||
| provider "oktapam" {} |
There was a problem hiding this comment.
Create base config string with group, resource group and project. Then, reuse that for creating and updating settings.
| func createTestAccServerCheckoutSettingsDeleteConfig(dgaName, resourceGroupName, projectName string) string { | ||
| return fmt.Sprintf(testAccServerCheckoutSettingsDeleteConfigFormat, dgaName, resourceGroupName, projectName) | ||
| } | ||
| func TestAccServerCheckoutSettingsSource(t *testing.T) { |
There was a problem hiding this comment.
Add some more test for below cases -
- First, removing exclude_list and changing it to include_list
- Then, remove include_list and use just checkout_required field
- Provide both include and exclude list
- Update project/resource group
oktapam/acctest.go
Outdated
There was a problem hiding this comment.
Moved all the acceptance test utility methods to this file instead of keeping those in provider_test.go
… provider factory
update the test case
guoliangye-okta
force-pushed
the
ssaxena/hackathon
branch
from
39a87ec
to
2e319d7
Compare
This PR is to support adding resources built using new TF Plugin Framework.
SDKV2 used for tf plugin development is designed for maintaining tf plugins that are compatible with Plugin Protocol version 5. Plugins need to communicate with Terraform CLI, protocol version 5 is supported by CLI version 0.12 and later. Protocol version 6 support tf cli version 1.0 or later.
To start using new TF Plugin Framework(https://developer.hashicorp.com/terraform/plugin/framework) we have two options:
If we go with Option 1, then will not be able to use some of the newer features like Nested Attributes
Going with option 2, will require upgrading tf cli version to 1.0+. I am going with option 2.