-
Notifications
You must be signed in to change notification settings - Fork 121
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
Reconcile terraform plugin framework resources without CLI #1086
Changes from all commits
e9f4718
85b2837
da8ea4b
7916fad
489ec36
5e1e54b
37fceff
9983676
bc3e24e
eb1990b
1c86003
a827b1c
8621373
f10b5a9
ec4cbf8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,9 +15,38 @@ import ( | |
"github.com/upbound/provider-aws/config/common" | ||
) | ||
|
||
// TerraformPluginFrameworkExternalNameConfigs contains all external | ||
// name configurations belonging to Terraform Plugin Framework | ||
// resources to be reconciled under the no-fork architecture for this | ||
// provider. | ||
var TerraformPluginFrameworkExternalNameConfigs = map[string]config.ExternalName{ | ||
// ec2 | ||
// | ||
// Imported by using the id: sgr-02108b27edd666983 | ||
"aws_vpc_security_group_egress_rule": vpcSecurityGroupRule(), | ||
// Imported by using the id: sgr-02108b27edd666983 | ||
"aws_vpc_security_group_ingress_rule": vpcSecurityGroupRule(), | ||
|
||
// cognito | ||
// | ||
// us-west-2_abc123/3ho4ek12345678909nh3fmhpko | ||
"aws_cognito_user_pool_client": cognitoUserPoolClient(), | ||
|
||
// simpledb | ||
// | ||
// SimpleDB Domains can be imported using the name | ||
"aws_simpledb_domain": config.NameAsIdentifier, | ||
|
||
// appconfig | ||
// | ||
// AppConfig Environments can be imported by using the environment ID and application ID separated by a colon (:) | ||
// terraform-plugin-framework | ||
"aws_appconfig_environment": appConfigEnvironment(), | ||
} | ||
|
||
// NoForkExternalNameConfigs contains all external name configurations | ||
// belonging to Terraform resources to be reconciled under the no-fork | ||
// architecture for this provider. | ||
// belonging to Terraform Plugin SDKv2 resources to be reconciled | ||
// under the no-fork architecture for this provider. | ||
var NoForkExternalNameConfigs = map[string]config.ExternalName{ | ||
// ACM | ||
// Imported using ARN that has a random substring: | ||
|
@@ -2651,22 +2680,7 @@ var NoForkExternalNameConfigs = map[string]config.ExternalName{ | |
"aws_fis_experiment_template": config.IdentifierFromProvider, | ||
} | ||
|
||
var CLIReconciledExternalNameConfigs = map[string]config.ExternalName{ | ||
// Imported by using the id: sgr-02108b27edd666983 | ||
"aws_vpc_security_group_egress_rule": vpcSecurityGroupRule(), | ||
// Imported by using the id: sgr-02108b27edd666983 | ||
"aws_vpc_security_group_ingress_rule": vpcSecurityGroupRule(), | ||
// Cognito User Pool clients can be imported using the user pool id and client id separated by a slash (/) | ||
// However, the terraform id is just the client id. | ||
"aws_cognito_user_pool_client": cognitoUserPoolClient(), | ||
// simpledb | ||
// | ||
// SimpleDB Domains can be imported using the name | ||
"aws_simpledb_domain": config.NameAsIdentifier, | ||
// AppConfig Environments can be imported by using the environment ID and application ID separated by a colon (:) | ||
// terraform-plugin-framework | ||
"aws_appconfig_environment": config.IdentifierFromProvider, | ||
} | ||
var CLIReconciledExternalNameConfigs = map[string]config.ExternalName{} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Let's remove this map as it's currently not needed. If we need in the future, then we can add. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lets remove this at CLI removal PR. |
||
|
||
// cognitoUserPoolClient | ||
// Note(mbbush) This resource has some unexpected behaviors that make it impossible to write a completely correct | ||
|
@@ -2805,6 +2819,24 @@ func vpcSecurityGroupRule() config.ExternalName { | |
return e | ||
} | ||
|
||
func appConfigEnvironment() config.ExternalName { | ||
// Terraform does not allow Environment ID to be empty. | ||
// Using a stub value to pass validation. | ||
e := config.IdentifierFromProvider | ||
e.SetIdentifierArgumentFn = func(base map[string]interface{}, externalName string) { | ||
if _, ok := base["environment_id"]; !ok { | ||
if externalName == "" { | ||
// must satisfy regular expression pattern: [a-z0-9]{4,7} | ||
base["environment_id"] = "tbdeid0" | ||
} | ||
if identifiers := strings.Split(externalName, ":"); len(identifiers) == 2 { | ||
base["environment_id"] = identifiers[0] | ||
} | ||
} | ||
} | ||
return e | ||
} | ||
|
||
func route() config.ExternalName { | ||
e := config.IdentifierFromProvider | ||
e.GetIDFn = func(_ context.Context, _ string, parameters map[string]interface{}, _ map[string]interface{}) (string, error) { | ||
|
@@ -3011,19 +3043,22 @@ func TemplatedStringAsIdentifierWithNoName(tmpl string) config.ExternalName { | |
return e | ||
} | ||
|
||
// ResourceConfigurator applies all external name configs | ||
// listed in the table NoForkExternalNameConfigs and | ||
// CLIReconciledExternalNameConfigs and sets the version | ||
// of those resources to v1beta1. For those resource in | ||
// NoForkExternalNameConfigs, it also sets | ||
// config.Resource.UseNoForkClient to `true`. | ||
// ResourceConfigurator applies all external name configs listed in | ||
// the table NoForkExternalNameConfigs, | ||
// CLIReconciledExternalNameConfigs, and | ||
// TerraformPluginFrameworkExternalNameConfigs and sets the version of | ||
// those resources to v1beta1. | ||
func ResourceConfigurator() config.ResourceOption { | ||
return func(r *config.Resource) { | ||
// if configured both for the no-fork and CLI based architectures, | ||
// no-fork configuration prevails | ||
e, configured := NoForkExternalNameConfigs[r.Name] | ||
// If an external name is configured for multiple architectures, | ||
// Terraform Plugin Framework takes precedence over Terraform | ||
// Plugin SDKv2, which takes precedence over CLI architecture. | ||
e, configured := TerraformPluginFrameworkExternalNameConfigs[r.Name] | ||
if !configured { | ||
e, configured = CLIReconciledExternalNameConfigs[r.Name] | ||
e, configured = NoForkExternalNameConfigs[r.Name] | ||
if !configured { | ||
e, configured = CLIReconciledExternalNameConfigs[r.Name] | ||
} | ||
Comment on lines
+3059
to
+3061
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please see the comment above regarding the CLI-based reconciliation map. |
||
} | ||
if !configured { | ||
return | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm not mistaken, the external-name syntax is not changing for the resource as we move from the CLI-based implementation to the native fw reconciliation. We are allowed to break APIs with the next release, and if we are breaking APIs, it's important for us to capture this in the release notes.