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

Reconcile terraform plugin framework resources without CLI #1086

Merged
merged 15 commits into from
Feb 1, 2024

Conversation

erhancagirici
Copy link
Collaborator

@erhancagirici erhancagirici commented Jan 16, 2024

Description of your changes

Reconciles terraform plugin framework resources without CLI using the new TerraformPluginFrameworkExternalClient no-fork architecture.

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/ec2/securitygroupingressrule.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/cognitoidp/userpoolclient.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/simpledb/domain.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/appconfig/environment.yaml"

1 similar comment
@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/appconfig/environment.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/ec2/securitygroupingressrule.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/cognitoidp/userpoolclient.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/simpledb/domain.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/appconfig/environment.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/ec2/v1beta1/securitygroupingressrule.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/cognitoidp/v1beta1/userpoolclient.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/simpledb/v1beta1/domain.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/appconfig/v1beta1/environment.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/ec2/v1beta1/securitygroupegressrule.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/ec2/v1beta1/securitygroupingressrule.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/cognitoidp/v1beta1/userpoolclient.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/simpledb/v1beta1/domain.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/appconfig/v1beta1/environment.yaml"

@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/elasticache/v1beta1/replicationgroup.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/ec2/v1beta1/securitygroupingressrule.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/cognitoidp/v1beta1/userpoolclient.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/simpledb/v1beta1/domain.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/appconfig/v1beta1/environment.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/elasticache/v1beta1/replicationgroup.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/elasticache/v1beta1/replicationgroup.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/appconfig/v1beta1/environment.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/simpledb/v1beta1/domain.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/cognitoidp/v1beta1/userpoolclient.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/ec2/v1beta1/securitygroupingressrule.yaml"

@ulucinar
Copy link
Collaborator

/test-examples="examples/ec2/v1beta1/securitygroupegressrule.yaml"

Copy link
Collaborator

@ulucinar ulucinar left a comment

Choose a reason for hiding this comment

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

Thanks @erhancagirici, lgtm. We may consider cleaning up the commit history.

// terraform-plugin-framework
"aws_appconfig_environment": config.IdentifierFromProvider,
}
var CLIReconciledExternalNameConfigs = map[string]config.ExternalName{}
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

lets remove this at CLI removal PR.

Comment on lines +3059 to +3061
if !configured {
e, configured = CLIReconciledExternalNameConfigs[r.Name]
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please see the comment above regarding the CLI-based reconciliation map.

//
// AppConfig Environments can be imported by using the environment ID and application ID separated by a colon (:)
// terraform-plugin-framework
"aws_appconfig_environment": appConfigEnvironment(),
Copy link
Collaborator

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.

mergenci and others added 15 commits February 1, 2024 10:37
Signed-off-by: Cem Mergenci <cmergenci@gmail.com>
…ources

Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
…ix aws_cognito_user_pool_client external name config

Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
… name config

Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Erhan Cagirici <erhan@upbound.io>
Signed-off-by: Alper Rifat Ulucinar <ulucinar@users.noreply.github.com>
@erhancagirici
Copy link
Collaborator Author

/test-examples="examples/ec2/v1beta1/securitygroupingressrule.yaml"

@erhancagirici erhancagirici merged commit 9b4cb47 into crossplane-contrib:main Feb 1, 2024
9 of 10 checks passed
@erhancagirici erhancagirici deleted the no-fork-v4.67.0-fw branch February 1, 2024 08:19
@mergenci mergenci mentioned this pull request Feb 26, 2024
6 tasks
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