Conversation
|
Warning Rate limit exceeded@dtoxvanilla1991 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 14 minutes and 21 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (13)
📒 Files selected for processing (75)
WalkthroughAdds a complete Terraform provider for Kinde: provider wiring, many resources and data sources (APIs, applications, connections, organizations, users, roles, permissions, assignments), client adapters/serde, extensive acceptance tests, examples, docs, and build/tooling/config files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant TF as Terraform CLI
participant Prov as Kinde Provider
participant Client as kinde-go Client
participant API as Kinde REST API
note over TF,Prov: Resource Create flow (generic)
TF->>Prov: Apply: create resource (plan)
Prov->>Client: Call SDK method (e.g., CreateApplication/CreateRole/CreateConnection)
Client->>API: HTTP request (POST/PUT/DELETE/GET)
API-->>Client: HTTP response (200/201/204 + body)
Client-->>Prov: SDK object/response
Prov->>Prov: expand/flatten, set diagnostics, mark sensitive fields
Prov-->>TF: Persist state (id, attributes, sensitive masked)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
♻️ Duplicate comments (1)
examples/resources/kinde_role/terraform.tf (1)
1-7: Verify provider source consistency.Same concern as other example files: the provider source is
"nxt-fwd/kinde"but the PR indicates a move to the official Kinde organization. This should be verified and updated consistently across all examples if needed.
🧹 Nitpick comments (7)
examples/resources/kinde_permission/terraform.tf (1)
9-25: Consider splitting into separate example patterns.This file demonstrates two different provider configuration approaches in a single file. Typically, Terraform examples show one pattern per file for clarity. Consider either:
- Keeping only the environment variable approach (lines 9-16) as the primary example
- Creating a separate example file for the explicit configuration pattern
Additionally, line 25 has trailing whitespace that should be removed.
examples/resources/kinde_application/provider.tf (1)
1-8: Consider renaming file for clarity (optional).The filename
provider.tfis misleading since this file contains a resource block rather than provider configuration. Consider renaming toresource.tforapplication.tffor clarity, though this is a minor naming convention suggestion for example files.docs/index.md (1)
31-31: Consider consistent spelling.Line 31 uses British spelling "organisation" while American spelling "organization" is used elsewhere in the codebase (e.g., resource names like
kinde_organization). Consider using "organization" for consistency.internal/serde/list.go (1)
19-23: Remove redundant variable declaration.The
diagsvariable is declared on line 20 but immediately shadowed by the short variable declaration on line 21, making the initial declaration unused.Apply this diff to simplify:
func FlattenStringList(ctx context.Context, input []string) (types.List, diag.Diagnostics) { - var diags diag.Diagnostics output, diags := types.ListValueFrom(ctx, types.StringType, input) return output, diags }examples/resources/kinde_role/resource.tf (1)
25-35: Consider adding a clarifying comment for resource references.The
full_examplereferenceskinde_permissionresources (create_users,edit_users,delete_users) that aren't defined in this file. While this demonstrates cross-resource referencing, users might benefit from a brief comment indicating these are defined elsewhere or linking to the permission resource examples.Example addition:
# Full example showing all features +# Note: This example references kinde_permission resources defined separately resource "kinde_role" "full_example" { name = "full_role"internal/provider/application_connection_resource.go (1)
142-153: Consider adding a comment explaining the no-op Update.Since both
application_idandconnection_idhaveRequiresReplaceplan modifiers, this Update method should never be called for actual changes. The current implementation is correct, but an explanatory comment would improve code maintainability.Example addition:
func (r *ApplicationConnectionResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { - // No updates are possible, all fields require replacement + // No-op: Both application_id and connection_id require replacement (RequiresReplace), + // so Terraform will never call Update for field changes. This method only handles + // non-field plan changes (e.g., timeouts). var plan applicationConnectionResourceModelinternal/provider/organization_resource_test.go (1)
55-61: Consider using consistent string formatting for safety.Line 58 uses
"%[1]s-updated"which manually concatenates the suffix within quotes. While this works for typical test names, using%[1]qwould be more consistent with the create config and safer if test names contain special characters.Apply this diff for consistency:
func testAccOrganizationResourceConfigUpdate(name string) string { return fmt.Sprintf(` resource "kinde_organization" "test" { - name = "%[1]s-updated" + name = %[1]q } -`, name) +`, name+"-updated") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
.github/dependabot.ymlis excluded by!**/*.yml.github/workflows/issue-comment-triage.ymlis excluded by!**/*.yml.github/workflows/lock.ymlis excluded by!**/*.yml.github/workflows/release.ymlis excluded by!**/*.yml.github/workflows/test.ymlis excluded by!**/*.yml.golangci.ymlis excluded by!**/*.yml.goreleaser.ymlis excluded by!**/*.ymlgo.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sumterraform-registry-manifest.jsonis excluded by!**/*.jsontools/go.modis excluded by!**/*.modtools/go.sumis excluded by!**/*.sum,!**/*.sum
📒 Files selected for processing (74)
.copywrite.hcl(1 hunks).env.example(1 hunks).github/CODEOWNERS(1 hunks).github/CODE_OF_CONDUCT.md(1 hunks)GNUmakefile(1 hunks)ORIGINAL.md(1 hunks)docs/data-sources/api.md(1 hunks)docs/data-sources/application.md(1 hunks)docs/data-sources/connections.md(1 hunks)docs/index.md(1 hunks)docs/resources/api.md(1 hunks)docs/resources/application.md(1 hunks)docs/resources/application_connection.md(1 hunks)docs/resources/connection.md(1 hunks)docs/resources/organization.md(1 hunks)docs/resources/organization_user.md(1 hunks)docs/resources/permission.md(1 hunks)docs/resources/role.md(1 hunks)docs/resources/user.md(1 hunks)docs/resources/user_role.md(1 hunks)examples/README.md(1 hunks)examples/data-sources/kinde_api/data-source.tf(1 hunks)examples/data-sources/kinde_api/terraform.tf(1 hunks)examples/data-sources/kinde_application/data-source.tf(1 hunks)examples/data-sources/kinde_application/terraform.tf(1 hunks)examples/provider/provider.tf(1 hunks)examples/provider/terraform.tf(1 hunks)examples/resources/kinde_api/provider.tf(1 hunks)examples/resources/kinde_api/terraform.tf(1 hunks)examples/resources/kinde_application/provider.tf(1 hunks)examples/resources/kinde_application/terraform.tf(1 hunks)examples/resources/kinde_organization_user/resource.tf(1 hunks)examples/resources/kinde_permission/resource.tf(1 hunks)examples/resources/kinde_permission/terraform.tf(1 hunks)examples/resources/kinde_role/complete_example.tf(1 hunks)examples/resources/kinde_role/resource.tf(1 hunks)examples/resources/kinde_role/terraform.tf(1 hunks)examples/resources/kinde_user_role/resource.tf(1 hunks)internal/provider/api_data_source.go(1 hunks)internal/provider/api_data_source_test.go(1 hunks)internal/provider/api_resource.go(1 hunks)internal/provider/api_resource_test.go(1 hunks)internal/provider/api_schema.go(1 hunks)internal/provider/application_connection_resource.go(1 hunks)internal/provider/application_connection_resource_test.go(1 hunks)internal/provider/application_data_source.go(1 hunks)internal/provider/application_data_source_test.go(1 hunks)internal/provider/application_resource.go(1 hunks)internal/provider/application_resource_test.go(1 hunks)internal/provider/application_schema.go(1 hunks)internal/provider/connection_resource.go(1 hunks)internal/provider/connection_resource_test.go(1 hunks)internal/provider/connections_data_source.go(1 hunks)internal/provider/integration_test.go(1 hunks)internal/provider/organization_resource.go(1 hunks)internal/provider/organization_resource_test.go(1 hunks)internal/provider/organization_user_resource.go(1 hunks)internal/provider/organization_user_schema.go(1 hunks)internal/provider/permission_resource.go(1 hunks)internal/provider/permission_resource_test.go(1 hunks)internal/provider/permission_schema.go(1 hunks)internal/provider/provider.go(1 hunks)internal/provider/provider_test.go(1 hunks)internal/provider/role_resource.go(1 hunks)internal/provider/role_resource_test.go(1 hunks)internal/provider/role_schema.go(1 hunks)internal/provider/user_resource.go(1 hunks)internal/provider/user_resource_test.go(1 hunks)internal/provider/user_role_resource.go(1 hunks)internal/provider/utils.go(1 hunks)internal/serde/list.go(1 hunks)main.go(1 hunks)templates/index.md.tmpl(1 hunks)tools/tools.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
internal/provider/provider_test.go (1)
internal/provider/provider.go (1)
New(132-138)
internal/provider/application_schema.go (1)
internal/serde/list.go (1)
FlattenStringList(19-23)
internal/provider/role_resource.go (1)
internal/provider/role_schema.go (1)
RoleResourceModel(16-22)
internal/provider/role_schema.go (1)
internal/serde/list.go (1)
FlattenStringList(19-23)
internal/provider/api_resource.go (1)
internal/provider/api_schema.go (1)
APIResourceModel(11-16)
internal/provider/permission_resource.go (1)
internal/provider/permission_schema.go (1)
PermissionResourceModel(11-16)
internal/provider/organization_user_resource.go (1)
internal/provider/organization_user_schema.go (1)
OrganizationUserResourceModel(12-18)
internal/provider/application_data_source.go (1)
internal/provider/application_schema.go (1)
ApplicationDataSourceModel(82-88)
main.go (1)
internal/provider/provider.go (1)
New(132-138)
internal/provider/api_data_source.go (1)
internal/provider/api_schema.go (1)
APIDataSourceModel(34-38)
internal/provider/provider.go (13)
internal/provider/api_resource.go (1)
NewAPIResource(24-26)internal/provider/application_resource.go (1)
NewApplicationResource(26-28)internal/provider/application_connection_resource.go (1)
NewApplicationConnectionResource(22-24)internal/provider/connection_resource.go (1)
NewConnectionResource(23-25)internal/provider/organization_resource.go (1)
NewOrganizationResource(22-24)internal/provider/organization_user_resource.go (1)
NewOrganizationUserResource(25-27)internal/provider/role_resource.go (1)
NewRoleResource(25-27)internal/provider/user_resource.go (1)
NewUserResource(26-28)internal/provider/permission_resource.go (1)
NewPermissionResource(23-25)internal/provider/user_role_resource.go (1)
NewUserRoleResource(26-28)internal/provider/api_data_source.go (1)
NewAPIDataSource(19-21)internal/provider/application_data_source.go (1)
NewApplicationDataSource(19-21)internal/provider/connections_data_source.go (1)
NewConnectionsDataSource(16-18)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 4-4: [UnorderedKey] The KINDE_AUDIENCE key should go before the KINDE_DOMAIN key
(UnorderedKey)
[warning] 5-5: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 5-5: [UnorderedKey] The KINDE_CLIENT_ID key should go before the KINDE_DOMAIN key
(UnorderedKey)
[warning] 6-6: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 6-6: [TrailingWhitespace] Trailing whitespace detected
(TrailingWhitespace)
[warning] 6-6: [UnorderedKey] The KINDE_CLIENT_SECRET key should go before the KINDE_DOMAIN key
(UnorderedKey)
🪛 GitHub Actions: Tests
internal/provider/connection_resource.go
[error] 128-128: golangci-lint: type connectionOptions is unused (unused)
🪛 GitHub Check: Build
internal/provider/application_resource_test.go
[failure] 128-128:
func testAccApplicationResourceConfig_ReducedConnections is unused (unused)
[failure] 114-114:
func testAccApplicationResourceConfig_ConnectionSetup is unused (unused)
internal/provider/connection_resource_test.go
[failure] 361-361:
func testAccConnectionResourceConfig_OnlyClientSecret is unused (unused)
[failure] 348-348:
func testAccConnectionResourceConfig_OnlyClientID is unused (unused)
internal/provider/organization_user_resource.go
[failure] 44-44:
func emptyListModifier.PlanModifyList is unused (unused)
[failure] 40-40:
func emptyListModifier.MarkdownDescription is unused (unused)
[failure] 36-36:
func emptyListModifier.Description is unused (unused)
[failure] 34-34:
type emptyListModifier is unused (unused)
internal/provider/connection_resource.go
[failure] 31-31:
Comment should end in a period (godot)
[failure] 128-128:
type connectionOptions is unused (unused)
🪛 LanguageTool
ORIGINAL.md
[grammar] ~11-~11: Use a hyphen to join words.
Context: ...k) platform. Terraform Plugin Framework specific guides are titled accordingly....
(QB_NEW_EN_HYPHEN)
[grammar] ~35-~35: Use a hyphen to join words.
Context: ...see the Go documentation for the most up to date information about using Go modul...
(QB_NEW_EN_HYPHEN)
[grammar] ~35-~35: Use a hyphen to join words.
Context: ... the Go documentation for the most up to date information about using Go modules....
(QB_NEW_EN_HYPHEN)
[style] ~57-~57: Consider a more concise word here.
Context: ...date documentation, run go generate. In order to run the full suite of Acceptance tests,...
(IN_ORDER_TO_PREMIUM)
docs/index.md
[grammar] ~8-~8: Ensure spelling is correct
Context: ...nde Identity Platform resources. --- # kinde Provider The Kinde provider is used to...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
ORIGINAL.md
5-5: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
5-5: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
60-60: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
60-60: Emphasis style
Expected: underscore; Actual: asterisk
(MD049, emphasis-style)
.github/CODE_OF_CONDUCT.md
5-5: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (48)
.github/CODE_OF_CONDUCT.md (1)
1-5: Verify Code of Conduct is appropriate for Kinde organization.The file currently references HashiCorp's Community Guidelines. For a Kinde-owned repository, verify that this is the intended governance approach. Consider whether Kinde should establish its own Code of Conduct or reference an alternative community standard (e.g., Contributor Covenant).
examples/resources/kinde_api/terraform.tf (1)
1-10: LGTM!Standard Terraform provider configuration with an empty provider block, which is appropriate for examples that rely on environment variable configuration.
examples/data-sources/kinde_api/terraform.tf (1)
1-10: LGTM!Standard Terraform provider configuration. The duplication across example directories is expected and allows each example to be self-contained and independently runnable.
.copywrite.hcl (1)
6-6: Verify the copyright year is correct.The copyright year is set to 2021, but this PR is from 2025 and appears to be establishing a new provider. Confirm whether this should be 2025 (or 2024 if development started then).
docs/resources/user.md (1)
1-41: LGTM!The generated documentation accurately describes the kinde_user resource schema with appropriate required, optional, and read-only attributes.
docs/resources/application_connection.md (1)
1-25: LGTM!The generated documentation clearly describes the kinde_application_connection resource with its required fields and composite ID structure.
internal/provider/utils.go (2)
12-18: LGTM!The
splitIDfunction correctly validates colon-separated IDs and provides clear error messages with the expected format.
20-28: LGTM!The
sortStringSlicefunction correctly creates a sorted copy while preserving the original slice and handles nil input gracefully.examples/data-sources/kinde_api/data-source.tf (1)
1-3: LGTM! Clean data source example.The example correctly demonstrates the kinde_api data source usage with a sample ID, which is appropriate for documentation purposes.
internal/provider/provider_test.go (1)
1-37: LGTM! Well-structured test scaffolding.The acceptance test setup follows Terraform Plugin Framework v6 patterns correctly. The environment variable validation in
testAccPreCheckensures all required credentials are present before tests run, and the use oft.Helper()ensures proper test failure line reporting.docs/data-sources/connections.md (1)
1-34: LGTM! Well-structured documentation.The generated documentation clearly describes the kinde_connections data source with appropriate schema details and filter parameter constraints.
docs/data-sources/api.md (1)
1-31: LGTM! Comprehensive documentation with external reference.The documentation provides a clear description, includes a link to official Kinde documentation, and demonstrates usage with an appropriate example. The schema clearly distinguishes required from read-only attributes.
examples/data-sources/kinde_application/terraform.tf (2)
9-10: LGTM! Appropriate provider configuration for examples.The empty provider block correctly relies on environment variables for configuration, which is the recommended approach for examples and aligns with the testAccPreCheck requirements in the test scaffolding.
1-7: Cannot confirm provider source change without PR context.The example files are consistently using
source = "nxt-fwd/kinde"(all 8 occurrences match), so there is no internal inconsistency to fix. However, the verification raised additional concerns:
- Web searches did not find either
nxt-fwd/kindeorkinde-oss/kindein the public Terraform Registry- The PR objectives mentioned in the original comment are not accessible in the repository context
- The codebase references
kinde-ossorganization in templates for contributing guidelines, but does not clarify the Terraform provider's official namespaceTo resolve this:
- Confirm the PR's migration objectives and whether the provider source requires updating
- Verify the official Kinde Terraform provider namespace (public registry, private registry, or internal location)
- If an update is needed, ensure all 8 files are updated consistently
examples/resources/kinde_role/terraform.tf (1)
9-25: LGTM! Excellent documentation of configuration options.The dual provider configuration pattern effectively demonstrates both environment variable-based configuration (recommended for security) and explicit configuration with an alias. The placeholder values in the explicit example are clearly identifiable as examples, and the comments provide helpful context for users.
examples/resources/kinde_application/provider.tf (1)
1-8: LGTM! Well-configured application resource example.The resource configuration demonstrates appropriate usage of the kinde_application resource with localhost URIs suitable for development examples. The array vs. string distinction for URIs aligns with typical OAuth patterns where some URIs allow multiple values.
examples/data-sources/kinde_application/data-source.tf (1)
1-3: LGTM! Clean data source example.The example correctly demonstrates the kinde_application data source usage with a sample ID, following the same clean pattern as other data source examples in this PR.
internal/provider/api_resource_test.go (1)
14-40: LGTM!The acceptance test follows Terraform Plugin Testing best practices with proper resource creation, attribute verification, and import state testing.
docs/resources/organization.md (1)
1-36: LGTM!The documentation is clear and complete with a well-structured schema describing the organization resource fields.
docs/data-sources/application.md (1)
1-33: LGTM!The data source documentation is clear with appropriate example usage and schema definition.
docs/resources/user_role.md (1)
1-66: LGTM!The documentation provides comprehensive examples covering various usage patterns (basic assignment, multiple assignments, and dependency-based scenarios) with a clear schema definition.
docs/resources/api.md (1)
21-21: Name field correctly implements ForceNew behavior—no changes needed.The schema already sets
stringplanmodifier.RequiresReplace()on the name attribute, which is the Terraform SDK v2 framework's equivalent ofForceNew: true. This ensures the resource is destroyed and recreated when the name changes, preventing users from encountering unexpected behavior. The documentation and implementation are consistent and correct.examples/resources/kinde_api/provider.tf (1)
1-4: LGTM!The example configuration is clear and demonstrates the basic usage of the
kinde_apiresource with required attributes.main.go (1)
30-33: Verify the provider registry address.The provider address is set to
"registry.opentofu.org/nxt-fwd/kinde", but the PR objectives indicate the repository has been moved to the official Kinde organization (kinde-oss). Please confirm whether:
- The address should be updated to reflect the new organization (e.g.,
registry.opentofu.org/kinde-oss/kinde)- The current address is intentional for backward compatibility or other reasons
docs/resources/permission.md (1)
1-65: LGTM!The documentation is comprehensive and well-structured, providing clear examples of various usage patterns including basic, with description, for_each iteration, and full configuration. The schema documentation is complete and accurate.
internal/serde/list.go (1)
13-17: LGTM!The
ExpandStringListfunction correctly converts a Terraform List to a Go string slice with proper diagnostic handling.docs/resources/connection.md (1)
1-38: LGTM!The documentation clearly describes the connection resource schema and includes an important security note about sensitive values being stored in state. The nested schema for OAuth2 options with sensitive fields is properly documented.
internal/provider/api_data_source_test.go (1)
12-39: LGTM!The acceptance test is well-structured and follows Terraform plugin testing best practices. It properly validates that the API data source can read attributes from a created resource.
examples/resources/kinde_permission/resource.tf (1)
1-33: LGTM!The examples effectively demonstrate various usage patterns for the permission resource, including basic usage, with description, for_each iteration for creating multiple related permissions, and a full example. This provides good guidance for users.
docs/resources/organization_user.md (1)
1-77: LGTM!The documentation is comprehensive and provides excellent examples covering basic membership, membership with roles, membership with permissions, and a complete example including user creation. The schema documentation clearly describes all fields and their types.
examples/provider/provider.tf (1)
1-6: LGTM!The provider configuration example is clear and includes helpful comments about environment variable alternatives. The placeholder values are appropriate for documentation purposes.
templates/index.md.tmpl (1)
1-16: LGTM!The documentation template follows Terraform plugin documentation conventions correctly. The structure and template syntax are appropriate.
internal/provider/application_connection_resource_test.go (2)
14-38: LGTM!The acceptance test is well-structured with proper dependency setup and cross-resource validation using
TestCheckResourceAttrPair. The ImportState test ensures the resource can be imported correctly.
40-62: LGTM!The test configuration helper correctly creates all required dependencies and properly formats the Terraform configuration using
%[1]qfor safe string interpolation.internal/provider/application_data_source_test.go (1)
12-36: LGTM!The data source test correctly creates a resource and validates that the data source can read it back with the expected attributes. The test structure follows Terraform testing best practices.
examples/resources/kinde_role/resource.tf (3)
1-5: LGTM!The basic role example demonstrates the minimal required configuration clearly.
7-12: LGTM!The example with description clearly demonstrates the optional description attribute.
14-23: LGTM!The permissions example uses placeholder IDs with clear comments indicating they should be replaced. This is appropriate for documentation.
internal/provider/application_connection_resource.go (6)
17-34: LGTM!The resource type definitions and constructor follow Terraform Plugin Framework patterns correctly. The interface assertions provide compile-time guarantees for required implementations.
36-62: LGTM!The schema is well-designed with appropriate plan modifiers. Both
application_idandconnection_idcorrectly useRequiresReplace, which makes sense for a join relationship resource.
64-79: LGTM!The Configure method correctly extracts and validates the provider client with appropriate error handling.
81-104: LGTM!The Create method correctly enables the connection and sets a composite ID that aligns with the ImportState format. Error handling is appropriate.
155-187: LGTM!The Delete method correctly disables the connection with appropriate error handling, and the ImportState method properly parses the composite ID format and populates all required state attributes.
106-140: Verify whether error type differentiation is expected across all resources or if uniform handling is intentional.The Read method treats all GetConnections errors uniformly, which is consistent with every other resource in this provider (user, role, permission, user_role). However, the concern about a deleted parent application is valid—if the app is deleted externally, GetConnections will fail with an error rather than gracefully removing the resource from state.
Since no error type checking exists anywhere in the codebase and the external kinde-go client's error types aren't inspected, this requires confirming:
- Should missing parent resources trigger state removal (as a drift detection pattern), or is uniform error propagation preferred?
- If differentiation is needed, would it apply to all resources or only nested ones like this?
internal/provider/permission_resource_test.go (2)
13-46: LGTM!The acceptance test provides comprehensive coverage of the permission resource lifecycle, including create, import, and update operations. All attributes are properly validated.
48-56: LGTM!The configuration helper correctly generates Terraform configuration with proper string quoting using
%[n]qformat specifiers.internal/provider/organization_resource_test.go (2)
11-45: LGTM!The organization resource test provides comprehensive coverage including validation of computed attributes like
created_on. The separate configuration helpers for create and update scenarios enhance test clarity.
47-53: LGTM!The create configuration helper properly uses
%[1]qfor safe string interpolation.
|
Hi @shirkevich, are you able to make changes to fix the code rabbit review comments? |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
internal/provider/connection_resource.go (1)
31-31: Fix all godoc comments to end with periods (godot linter).Multiple comments are missing periods, causing CI build failures. The godot linter requires all exported type and method comments to end with punctuation.
🔎 Proposed fix
-// ConnectionOptionsModel represents OAuth2 connection options +// ConnectionOptionsModel represents OAuth2 connection options. type ConnectionOptionsModel struct { ClientID types.String `tfsdk:"client_id" json:"client_id,omitempty"` ClientSecret types.String `tfsdk:"client_secret" json:"client_secret,omitempty"` } -// IsEmpty returns true if both fields are null or empty +// IsEmpty returns true if both fields are null or empty. func (m *ConnectionOptionsModel) IsEmpty() bool { -// Validate ensures both fields are either both set or both null +// Validate ensures both fields are either both set or both null. func (m *ConnectionOptionsModel) Validate() error { -// ToAPIOptions converts the model to API options +// ToAPIOptions converts the model to API options. func (m *ConnectionOptionsModel) ToAPIOptions() connections.SocialConnectionOptions { -// ConnectionResourceModel represents the resource model +// ConnectionResourceModel represents the resource model. type ConnectionResourceModel struct { -// Equal compares two ConnectionResourceModel instances +// Equal compares two ConnectionResourceModel instances. func (m *ConnectionResourceModel) Equal(other *ConnectionResourceModel) bool { -// Plan modifier for options +// Plan modifier for options. type optionsEmptyPreserveModifier struct{}Also applies to: 37-37, 48-48, 63-63, 79-79, 88-88, 127-127
🧹 Nitpick comments (9)
internal/provider/api_schema.go (1)
32-42: Consider adding a nil guard for defensive coding.The function now correctly maps
IsManagementAPI(line 39), addressing the prior review. However, for added robustness, consider checking ifresourceis nil before dereferencing its fields, even though the current usage pattern shouldn't pass nil values.🔎 Optional defensive nil check
func flattenAPIResource(resource *apis.API) APIResourceModel { + if resource == nil { + return APIResourceModel{} + } model := APIResourceModel{ ID: types.StringValue(resource.ID), Name: types.StringValue(resource.Name), Audience: types.StringValue(resource.Audience), } model.IsManagementAPI = types.BoolValue(resource.IsManagementAPI) return model }examples/resources/kinde_user_role/resource.tf (1)
14-18: Duplicate role assignment in example.Both
basic_assignment(lines 14-18) andexample_assignment(lines 50-54) assign the same user (example_user) to the same role (example_role) with the same organization code. This redundancy may confuse users copying this example. Consider removing one or documenting the intent.🔎 Suggested fix: Remove redundant assignment
-resource "kinde_user_role" "example_assignment" { - user_id = kinde_user.example_user.id - role_id = kinde_role.example_role.id - organization_code = "org_123" # Replace with your organization code -}Also applies to: 50-54
internal/provider/connections_data_source.go (1)
125-147: Consider validating the filter parameter.Invalid filter values (e.g.,
"invalid") silently fall through to thedefaultcase and return all connections. Consider adding validation in the schema or returning a diagnostic error for unrecognized filter values.🔎 Suggested validation
filter := "all" if !data.Filter.IsNull() { filter = data.Filter.ValueString() + if filter != "builtin" && filter != "custom" && filter != "all" { + resp.Diagnostics.AddError( + "Invalid Filter Value", + fmt.Sprintf("Filter must be one of: builtin, custom, all. Got: %s", filter), + ) + return + } }Alternatively, use a
schema.StringValidatorin the schema definition.internal/provider/role_resource.go (1)
172-178: Fix godot lint: comment should end with a period.The static analysis tool flagged that the comment on line 172 should end with a period.
🔎 Suggested fix
-// Helper function to sort permissions without modifying original +// Helper function to sort permissions without modifying original. func sortPermissions(permissions []string) []string {internal/provider/organization_resource.go (1)
265-272: Consider handling 404 in Read for out-of-band deletions.If an organization is deleted outside of Terraform, the Read method will return an error rather than removing the resource from state. This is inconsistent with
RoleResource.Readwhich handles this case gracefully.🔎 Suggested fix
organization, err := r.client.Get(ctx, state.Code.ValueString()) if err != nil { + if isNotFoundError(err) { + resp.State.RemoveResource(ctx) + return + } resp.Diagnostics.AddError( "Error Reading Organization", fmt.Sprintf("Could not read organization code %s: %s", state.Code.ValueString(), err), ) return }internal/provider/user_resource.go (1)
335-342: Consider handling 404 in Read for out-of-band deletions.If a user is deleted outside of Terraform, Read returns an error. Consider gracefully removing from state like
RoleResourcedoes.🔎 Suggested fix
user, err := r.client.Get(ctx, state.ID.ValueString()) if err != nil { + if isNotFoundError(err) { + resp.State.RemoveResource(ctx) + return + } resp.Diagnostics.AddError( "Error Reading User", fmt.Sprintf("Could not read user ID %s: %s", state.ID.ValueString(), err), ) return }internal/provider/user_resource_test.go (1)
16-16: Fix godot lint: comments should end with periods.Static analysis flagged that comments on lines 16 and 429 should end with periods.
🔎 Suggested fixes
-// TestUserResource_FiltersOAuthIdentities is a simple unit test for the OAuth filtering logic +// TestUserResource_FiltersOAuthIdentities is a simple unit test for the OAuth filtering logic. func TestUserResource_FiltersOAuthIdentities(t *testing.T) {-// TestUserResource_SortsIdentitiesConsistently tests that identities are sorted consistently +// TestUserResource_SortsIdentitiesConsistently tests that identities are sorted consistently. func TestUserResource_SortsIdentitiesConsistently(t *testing.T) {Also applies to: 429-429
internal/provider/connection_resource.go (1)
200-215: Add nil check for client.Connections for consistency.The
ApplicationResource.Configuremethod (in application_resource.go, lines 123-129) includes a nil check forclient.Applicationsbefore assignment. For consistency and defensive programming, consider adding the same check here.🔎 Proposed addition
client, ok := req.ProviderData.(*kinde.Client) if !ok { resp.Diagnostics.AddError( "Unexpected Resource Configure Type", fmt.Sprintf("Expected *kinde.Client, got: %T. Please report this issue to the provider developers.", req.ProviderData), ) return } + if client.Connections == nil { + resp.Diagnostics.AddError( + "Unconfigured Connections Client", + "Expected configured connections client. Please report this issue to the provider developers.", + ) + return + } + r.client = client.Connections }internal/provider/organization_user_resource.go (1)
219-259: Consider optimizing role reconciliation with sets.The current role diff logic uses nested loops (lines 220-227 and 241-248), resulting in O(n²) complexity. For small role counts this is acceptable, but you could optimize to O(n) by using map-based lookups:
// Create sets for efficient lookup currentSet := make(map[string]bool) for _, id := range currentRoleIDs { currentSet[id] = true } desiredSet := make(map[string]bool) for _, id := range desiredRoles { desiredSet[id] = true } // Remove roles not in desired set for roleID := range currentSet { if !desiredSet[roleID] { // remove role } } // Add roles not in current set for roleID := range desiredSet { if !currentSet[roleID] { // add role } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.vscode/settings.jsonis excluded by!**/*.json
📒 Files selected for processing (27)
.copywrite.hcl.github/CODEOWNERS.github/CODE_OF_CONDUCT.md.gitignoreGNUmakefiledocs/resources/application.mddocs/resources/organization_user.mddocs/resources/role.mdexamples/README.mdexamples/resources/kinde_organization_user/resource.tfexamples/resources/kinde_role/complete_example.tfexamples/resources/kinde_user_role/resource.tfinternal/provider/api_resource.gointernal/provider/api_schema.gointernal/provider/application_resource.gointernal/provider/application_resource_test.gointernal/provider/connection_resource.gointernal/provider/connection_resource_test.gointernal/provider/connections_data_source.gointernal/provider/organization_resource.gointernal/provider/organization_user_resource.gointernal/provider/organization_user_schema.gointernal/provider/permission_resource.gointernal/provider/role_resource.gointernal/provider/user_resource.gointernal/provider/user_resource_test.gointernal/provider/utils.go
✅ Files skipped from review due to trivial changes (2)
- .github/CODE_OF_CONDUCT.md
- docs/resources/role.md
🚧 Files skipped from review as they are similar to previous changes (9)
- examples/README.md
- internal/provider/organization_user_schema.go
- examples/resources/kinde_organization_user/resource.tf
- .github/CODEOWNERS
- examples/resources/kinde_role/complete_example.tf
- internal/provider/application_resource_test.go
- GNUmakefile
- .copywrite.hcl
- docs/resources/application.md
🧰 Additional context used
🧬 Code graph analysis (4)
internal/provider/organization_user_resource.go (1)
internal/provider/organization_user_schema.go (1)
OrganizationUserResourceModel(10-15)
internal/provider/role_resource.go (1)
internal/provider/role_schema.go (1)
RoleResourceModel(16-22)
internal/provider/permission_resource.go (1)
internal/provider/permission_schema.go (1)
PermissionResourceModel(11-16)
internal/provider/api_resource.go (1)
internal/provider/api_schema.go (1)
APIResourceModel(11-16)
🪛 GitHub Actions: Tests
internal/provider/connection_resource.go
[error] 37-37: Comment should end in a period (godot).
🪛 GitHub Check: Build
internal/provider/connection_resource.go
[failure] 127-127:
Comment should end in a period (godot)
[failure] 88-88:
Comment should end in a period (godot)
[failure] 79-79:
Comment should end in a period (godot)
[failure] 63-63:
Comment should end in a period (godot)
[failure] 48-48:
Comment should end in a period (godot)
[failure] 37-37:
Comment should end in a period (godot)
internal/provider/role_resource.go
[failure] 172-172:
Comment should end in a period (godot)
internal/provider/user_resource_test.go
[failure] 429-429:
Comment should end in a period (godot)
[failure] 16-16:
Comment should end in a period (godot)
🔇 Additional comments (27)
.gitignore (1)
24-26: LGTM!The additions are syntactically correct and appropriately grouped with other tool-specific ignore patterns. However, I notice Serena MCP is not mentioned in the PR objectives. Please confirm this is an intentional tooling addition to the development workflow.
internal/provider/api_schema.go (3)
11-16: Well-defined model structure.The
APIResourceModelcorrectly includes all necessary fields includingIsManagementAPI, which addresses the previous review concern about this field being missing from the mapping.
18-30: Past review concern successfully addressed.The
expandAPIResourceModelfunction now properly mapsIsManagementAPIfrom the model to the API struct (lines 25-27), with appropriate null/unknown guards. This directly resolves the previous review comment.
44-64: Data source model and helpers look correct.The
APIDataSourceModelappropriately excludesIsManagementAPI(data sources typically only expose required query fields), and both conversion helpers follow standard patterns.internal/provider/api_resource.go (8)
19-26: LGTM - Standard resource initialization.The interface compliance checks and constructor follow Terraform provider best practices.
36-62: Well-designed schema with clear documentation.The schema correctly marks
nameandaudienceas requiring replacement (lines 49, 54), which aligns with the API limitation documented at line 47 and the Update method behavior.
64-79: Standard configuration wiring.The Configure method properly type-asserts and wires up the API client with appropriate error handling.
81-117: Standard create-then-fetch pattern.The Create method follows the common Terraform provider pattern of creating the resource and then fetching it to populate computed fields (lines 104-112). This is necessary to retrieve
is_management_apifrom the API.
145-150: Appropriate handling of unsupported operation.The Update method correctly returns an error with a clear explanation, consistent with the schema's
RequiresReplacemodifiers and the API limitation documented in the schema.
152-171: Delete is properly idempotent.The Delete method correctly handles 404 errors by returning without error (lines 162-164), making the operation idempotent as recommended in the previous review.
173-175: Standard import implementation.The ImportState method uses the standard passthrough pattern for ID-based imports.
119-143: Past review concern successfully addressed.The Read method properly handles 404 errors by removing the resource from state (lines 129-132), which resolves the previous review comment about hard-failing on out-of-band deletions. The
isNotFoundErrorfunction correctly identifies HTTP 404 responses from the Kinde API by checking the status code with proper error unwrapping. This is the correct Terraform provider pattern.internal/provider/utils.go (1)
1-74: Well-structured utility helpers.The utility functions are well-implemented:
sortStringSlicecorrectly creates a copy before sorting to avoid side effectshasStatusCodeproperly traverses the error chain usingerrors.UnwrapstatusCodeFromErrorhandles both pointer and non-pointer struct types via reflectionThe reflection-based approach for extracting status codes is flexible and handles the SDK's error types gracefully.
internal/provider/connections_data_source.go (1)
110-116: Good defensive nil check for the client.The nil client check properly prevents a panic and provides a clear diagnostic message. This addresses the previous review feedback.
internal/provider/permission_resource.go (2)
249-273: Good pagination implementation.The
listAllPermissionshelper properly handles pagination by looping untilNextTokenis empty. This addresses the previous review concern about permissions being dropped when exceeding the page size.
140-166: Read fallback to name/key lookup is reasonable but has edge case.The fallback to match by name and key (lines 153-162) when ID lookup fails is helpful for drift detection. However, if a permission is renamed, the resource will be removed from state rather than detecting the update. This is acceptable behavior for most use cases but worth noting.
internal/provider/role_resource.go (1)
188-199: Good handling of out-of-band deletions.The Read method now properly handles 404 errors by removing the resource from state instead of returning an error. This allows Terraform to correctly handle roles deleted outside of Terraform.
internal/provider/organization_resource.go (1)
162-199: Good pattern for applying optional attributes on create.The implementation now correctly applies optional attributes by calling
buildOrganizationUpdateParamsafter the initial create. This addresses the previous review concern about drift on first apply.internal/provider/user_resource.go (1)
637-657: Good identity removal implementation.The Update method now properly removes identities that are in state but not in the plan. This addresses the previous review concern about identity removals not being synced.
internal/provider/user_resource_test.go (2)
78-150: Comprehensive acceptance test coverage.The
TestAccUserResource_ComplexAttributestest thoroughly exercises the create/update/delete lifecycle with multiple identity types and theis_suspendedflag. Good coverage of edge cases.
513-544: Good test for is_suspended update behavior.This test validates that
is_suspendedcan be set during update even though it cannot be set during create. This documents the expected behavior well.internal/provider/connection_resource_test.go (1)
1-371: Comprehensive test coverage, well structured.The test suite thoroughly covers the connection resource lifecycle including:
- OAuth2 connection creation, updates, and import
- Sensitive field handling (transitions between empty and populated options)
- Import state verification with appropriate warnings
- Edge cases like empty-to-populated transitions
All test helpers are utilized and the test structure follows Terraform testing best practices.
docs/resources/organization_user.md (1)
1-67: LGTM! Well-structured documentation.The documentation is clear, comprehensive, and includes helpful examples covering basic usage, role assignment, and full user creation workflows. The note about permission limitations provides important context for users.
internal/provider/organization_user_resource.go (4)
1-86: LGTM! Standard provider setup.The package structure, imports, interface implementations, and schema definition follow Terraform Plugin Framework best practices. The schema correctly uses
RequiresReplace()for organization_code and user_id, ensuring a new membership is created when these change.
299-313: LGTM! Import state handling is correct.The ImportState implementation properly parses the composite ID format and sets all required attributes. The format matches the ID constructed in Create (line 136:
organization_code:user_id).
16-17: The dependency versions in the code are current and secure. The codebase already usesgithub.com/nxt-fwd/kinde-gov0.0.8, which is the latest available version (released April 1, 2025). Whilegithub.com/hashicorp/terraform-plugin-frameworkv1.16.1 is available (released September 29, 2025), upgrading from v1.14.0 is optional—there are no breaking changes between these versions, though v1.16.0+ requires Go 1.24 (the project currently uses Go 1.23.0). No security advisories exist for either dependency.Likely an incorrect or invalid review comment.
267-297: The Delete method correctly uses the low-levelNewRequestandDoRequestAPI because the Kinde SDK does not provide a high-level method for removing users from organizations. This approach is appropriate and consistent with other SDK clients in the codebase (e.g.,permission_resource.go) that lack higher-level methods. No refactoring is needed.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
internal/provider/user_resource.go (1)
799-811: ✅ Past issue resolved: OAuth2 identities now filtered in ImportState.The ImportState method now correctly filters OAuth2 identities (lines 800-803), matching the behavior in Read and Update. This prevents import drift where OAuth2 identities would be imported but then removed on the next refresh. Good fix!
🧹 Nitpick comments (2)
internal/provider/organization_user_resource.go (1)
253-293: Consider optimizing role reconciliation with maps.The current role reconciliation uses nested loops (O(n×m) complexity) to find roles to add and remove. For typical role counts this is fine, but you could optimize to O(n+m) using maps:
// Build map of desired roles desired := make(map[string]bool) for _, roleID := range desiredRoles { desired[roleID] = true } // Remove roles not in desired set for _, roleID := range currentRoleIDs { if !desired[roleID] { // remove role } } // Build map of current roles current := make(map[string]bool) for _, roleID := range currentRoleIDs { current[roleID] = true } // Add roles not in current set for _, roleID := range desiredRoles { if !current[roleID] { // add role } }internal/provider/application_resource.go (1)
61-61: Clarify the MarkdownDescription.The phrase "Currently, there is no way to change this via the management application" is ambiguous. Consider revising to explicitly state whether this refers to a Kinde API limitation or the Terraform provider's design choice.
🔎 Suggested improvement
- "name": schema.StringAttribute{ - MarkdownDescription: "Name of the application. Currently, there is no way to change this via the management application.", + "name": schema.StringAttribute{ + MarkdownDescription: "Name of the application. Cannot be changed after creation - requires resource replacement.", Required: true, PlanModifiers: []planmodifier.String{stringplanmodifier.RequiresReplace()}, },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
internal/provider/application_resource.gointernal/provider/connection_resource.gointernal/provider/organization_user_resource.gointernal/provider/user_resource.go
🧰 Additional context used
🧬 Code graph analysis (1)
internal/provider/organization_user_resource.go (1)
internal/provider/organization_user_schema.go (1)
OrganizationUserResourceModel(10-15)
🪛 GitHub Actions: Tests
internal/provider/connection_resource.go
[error] 37-37: golangci-lint: Comment should end in a period (godot)
🪛 GitHub Check: Build
internal/provider/connection_resource.go
[failure] 88-88:
Comment should end in a period (godot)
[failure] 79-79:
Comment should end in a period (godot)
[failure] 63-63:
Comment should end in a period (godot)
[failure] 48-48:
Comment should end in a period (godot)
[failure] 37-37:
Comment should end in a period (godot)
🔇 Additional comments (24)
internal/provider/organization_user_resource.go (3)
110-142: Excellent idempotency implementation.The Create method now properly handles the non-idempotent
AddUsersAPI by:
- Pre-checking membership with
GetUserRoles(line 112) before attempting to add the user- Gracefully treating HTTP 409 Conflict as success when concurrent creation occurs (line 132)
- Implementing rollback logic (lines 149-155) to remove membership if role assignment fails
This addresses the critical issue raised in previous reviews about partial-failure scenarios and makes the resource safe for retries.
182-193: Correct not-found handling implemented.The Read method now properly handles the case where a user has been removed from the organization outside of Terraform (lines 184-186). By removing the resource from state when a not-found error is detected, Terraform can correctly detect and reconcile manual deletions during the next plan/apply cycle.
This addresses the major issue raised in previous reviews.
114-114: All helper functions are properly defined in the provider package (internal/provider/utils.go):
isNotFoundError(line 33)hasStatusCode(line 37)splitID(line 15)No issues found.
internal/provider/user_resource.go (6)
146-153: Good validation: is_suspended constraint on create.The validation preventing
is_suspended=trueduring creation is correct and provides a clear error message to guide users.
187-193: Good validation: email identity requirement.The validation ensuring at least one email identity is provided follows Kinde's requirements and will prevent runtime errors.
256-283: Solid identity handling with OAuth2 filtering and deterministic sorting.The identity conversion logic correctly:
- Filters out OAuth2 identities from state (line 258)
- Preserves user-specified types from the plan (lines 262-266)
- Sorts deterministically to avoid spurious diffs (lines 277-283)
This ensures stable, predictable state management.
360-393: Good pattern: preserving user-specified identity types.The Read method correctly preserves identity types from existing state when available, ensuring that user-specified types (e.g., "email" vs the API's potentially different representation) are maintained across refreshes.
637-657: Excellent: identity reconciliation with OAuth2 preservation.The Update method correctly reconciles identities by removing managed identities that are no longer desired (lines 637-657) while preserving OAuth2 identities (filtered earlier at lines 550-560). This ensures Terraform can converge and resolves the critical drift issue mentioned in past reviews.
The defensive nil check for
identitiesClient(lines 642-648) provides a clear error message if the client isn't configured.
577-596: Good validation: email identity with OAuth2 consideration.The validation correctly considers both planned and OAuth2 identities when checking for at least one email (lines 577-588), ensuring the requirement is met even when OAuth2 identities are preserved but excluded from state.
internal/provider/application_resource.go (7)
1-28: LGTM!The package structure, imports, and resource type declarations follow the standard Terraform Plugin Framework patterns correctly.
34-44: LGTM!The model struct is well-defined with appropriate Terraform types and tfsdk tags for proper state management.
109-133: LGTM!The Configure method properly validates the provider data with comprehensive error handling and informative diagnostics.
135-214: LGTM!The Create method correctly implements a two-phase approach: first creating the application with required fields, then updating with optional settings. The implementation properly excludes
Namefrom the update parameters, consistent with the schema'sRequiresReplace()modifier.
216-264: LGTM with awareness note.The Read method correctly handles all application fields. The logic at Lines 254-260 preserves
logout_urisandredirect_urisfrom state since the API doesn't return them. This is the appropriate approach given the API limitation, though it means drift detection won't work for these fields if removed from configuration.
266-326: LGTM! Previous issue resolved.The Update method correctly excludes
Namefrom the update parameters, which is consistent with the schema marking it withRequiresReplace(). This resolves the inconsistency flagged in the previous review.
328-348: LGTM!Both Delete and ImportState methods follow standard Terraform Plugin Framework patterns with appropriate error handling.
internal/provider/connection_resource.go (8)
1-29: LGTM!Package structure, imports, and type declarations are properly organized following Terraform Plugin Framework conventions.
120-159: LGTM!The schema correctly marks
client_idandclient_secretas sensitive and documents the security model. The custom plan modifier is appropriately applied to preserve sensitive values that the API doesn't return.
161-176: LGTM!The Configure method properly validates and configures the connections client with appropriate error handling.
178-224: LGTM with security documentation.The Create method correctly handles sensitive connection options by storing them in state and documenting reliance on Terraform's state encryption. This is the standard approach for managing sensitive values in Terraform providers.
226-316: LGTM!The Read, Update, and Delete methods are properly implemented with correct handling of sensitive options preservation and appropriate error handling throughout.
318-329: LGTM! Excellent user experience.The ImportState method properly warns users that sensitive values cannot be imported and must be configured manually. This is excellent documentation of expected behavior.
331-358: LGTM!The helper method comprehensively covers all OAuth2 strategy types supported by the Kinde API with proper error handling for unsupported strategies.
360-382: LGTM!The ValidateConfig method properly validates that OAuth2 connections have both client_id and client_secret configured, ensuring data integrity before resource operations.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @internal/provider/connection_resource.go:
- Around line 142-156: The schema's "options" SingleNestedAttribute currently
says state encryption is relied upon but isn't explicit enough; update the
MarkdownDescription for the "options" attribute to include a clear, prominent
warning that Terraform state is not encrypted by default and that storing
"client_id" and "client_secret" in state requires using an encrypted backend
(e.g., Terraform Cloud, S3 with SSE/KMS, etc.), and add a short recommended
configuration example or link to provider/backend docs; also add the same
explicit warning to the provider/resource documentation pages so users
understand they must configure an encrypted backend when using OAuth2 options.
In @internal/provider/user_resource.go:
- Around line 80-83: The organization_code attribute is sent on create (see
plan.OrganizationCode.ValueString()) but never persisted to state; inspect the
API user response in your Create, Read, and ImportState implementations
(resource Create/Read/Update/ImportState methods) — if the API returns
organization code, set the schema attribute (organization_code) from the API
response after Create and in Read/ImportState so state is populated; if the API
does not return it (write-only), update the schema definition for
organization_code to mark it as ForceNew (so changes cause replacement) and
adjust any validation/comments accordingly.
🧹 Nitpick comments (3)
internal/provider/connection_resource.go (1)
360-382: Consider validating that non-OAuth2 strategies don't set options.The current validation only checks OAuth2 option validity but doesn't prevent users from specifying options for non-OAuth2 strategies. While the API will likely reject such configurations, catching this in validation would provide better error messages.
🔎 Suggested enhancement
func (r *ConnectionResource) ValidateConfig(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { var data ConnectionResourceModel resp.Diagnostics.Append(req.Config.Get(ctx, &data)...) if resp.Diagnostics.HasError() { return } // Validate strategy if !data.Strategy.IsNull() { strategy := connections.Strategy(data.Strategy.ValueString()) if strings.HasPrefix(string(strategy), "oauth2:") { // Validate options if present if data.Options != nil { if err := data.Options.Validate(); err != nil { resp.Diagnostics.AddError( "Invalid Options Configuration", err.Error(), ) } } + } else { + // Non-OAuth2 strategies should not have options + if data.Options != nil && !data.Options.IsEmpty() { + resp.Diagnostics.AddError( + "Invalid Options Configuration", + fmt.Sprintf("Strategy %s does not support options", strategy), + ) + } } } }internal/provider/user_resource.go (2)
171-209: Consider validating that OAuth2 identities are not in the configuration.The resource correctly filters OAuth2 identities when reading from the API (lines 266-268, 391-393, 701-703, 809-811), but it doesn't prevent users from specifying them in their Terraform configuration.
If a user adds an
oauth2:*identity to their config:
- During Create: The identity might be sent to the API (lines 184-201 don't filter OAuth2), but then filtered from state (lines 266-268), causing perpetual drift
- During Update: The identity is silently skipped (lines 617-619), which could confuse users
🔎 Recommended validation to add
Add validation after parsing identities:
diags = plan.Identities.ElementsAs(ctx, &identities, false) resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { return } + +// Validate that OAuth2 identities are not in the plan +for _, identity := range identities { + if strings.HasPrefix(identity.Type, "oauth2:") { + resp.Diagnostics.AddError( + "Invalid Identity Type", + fmt.Sprintf("OAuth2 identities (type: %s) are managed automatically and cannot be specified in Terraform configuration. Please remove this identity from your configuration.", identity.Type), + ) + } +} +if resp.Diagnostics.HasError() { + return +} hasEmail := falseApply similar validation in the Update method around line 579.
781-840: Consider populatingis_suspendedduring import for complete state representation.The
ImportStatemethod sets most user attributes (id, first_name, last_name, created_on, updated_on, identities) but omitsis_suspendedeven though it's available in the API response.This creates an inconsistency:
- If a user is suspended in Kinde and you import the resource,
is_suspendedwon't appear in state- The
Readmethod (lines 434-436) only updatesis_suspendedif it was already in state (not null)- This means imported resources won't reflect the actual suspension status unless users explicitly add
is_suspendedto their configuration🔎 Optional improvement to populate is_suspended on import
resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("id"), user.ID)...) resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("first_name"), user.FirstName)...) resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("last_name"), user.LastName)...) +resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("is_suspended"), user.IsSuspended)...) resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("created_on"), user.CreatedOn.String())...) resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("updated_on"), user.UpdatedOn.String())...) resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("identities"), identitiesSet)...)This gives imported resources a complete state representation that matches the API.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/provider/connection_resource.gointernal/provider/user_resource.go
🔇 Additional comments (5)
internal/provider/connection_resource.go (2)
318-329: Well-designed import experience.The warning about sensitive values not being imported is excellent UX. This clearly communicates the limitation and sets proper expectations for users importing existing connections.
14-15: No action needed: v0.0.8 is the current stable version with no known security advisories.The kinde-go dependency is already pinned to v0.0.8, which is the latest released version as of April 2025. There are no available updates and no known security advisories for this version.
internal/provider/user_resource.go (3)
264-268: OAuth2 identity filtering is consistently implemented.The resource correctly excludes OAuth2 identities from Terraform state across all lifecycle methods (Create, Read, Update, ImportState) while preserving them during updates (lines 558-567, 610-612). This design allows OAuth-based logins to work without causing Terraform drift, since these identities are externally managed.
The reconciliation logic in Update properly handles identity additions and removals while preserving OAuth2 identities (lines 645-665), which resolves the critical issue from previous reviews.
Also applies to: 391-393, 701-703, 809-811
146-153: Validation logic properly enforces API constraints.The validation approach is well-designed:
- Create-time validations (lines 155-169): Ensures
first_nameandlast_nameare provided initially, which allows them to be Optional in the schema while still required for user creation- Suspension constraint (lines 146-153): Prevents setting
is_suspended=trueon create, with a clear error message directing users to create first then update- Update-time protections (lines 475-489): Prevents unsetting names once they're set, matching the Kinde API limitation
This pattern of Optional schema + explicit validation gives users flexibility while enforcing API requirements.
Also applies to: 155-169, 475-489
285-291: Deterministic identity sorting prevents spurious diffs.The consistent sorting of identities by type and then by value across all lifecycle methods ensures that Terraform won't detect false changes due to ordering differences. This is a best practice for managing set-based attributes.
Also applies to: 412-418, 720-726
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @internal/provider/role_resource.go:
- Around line 391-414: The ImportState implementation in RoleResource
(ImportState) currently calls resp.State.Set(ctx, &state) and ignores the
returned diagnostics; change it to capture the returned diag variable (e.g.,
diags := resp.State.Set(ctx, &state)), append those diagnostics into
resp.Diagnostics (resp.Diagnostics.Append(diags...)) and return early if
resp.Diagnostics.HasError() is true, mirroring the pattern used in
Create/Read/Update to ensure import errors are propagated.
🧹 Nitpick comments (1)
internal/provider/role_resource_test.go (1)
249-256: Simplify permissions string building withstrings.Join.The manual loop can be replaced with a more idiomatic approach.
🔎 Proposed simplification
+import "strings" + func testAccRoleResourceConfig_WithPermissions(name, key string, permissions []string) string { description := "Test role with permissions" if len(permissions) == 0 { return fmt.Sprintf(` resource "kinde_role" "test" { name = %q key = %q description = %q } `, name, key, description) } - permissionsStr := "[" - for i, p := range permissions { - if i > 0 { - permissionsStr += ", " - } - permissionsStr += fmt.Sprintf(`"%s"`, p) - } - permissionsStr += "]" + quoted := make([]string, len(permissions)) + for i, p := range permissions { + quoted[i] = fmt.Sprintf(`"%s"`, p) + } + permissionsStr := "[" + strings.Join(quoted, ", ") + "]" return fmt.Sprintf(`
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.golangci.ymlis excluded by!**/*.yml
📒 Files selected for processing (3)
internal/provider/role_resource.gointernal/provider/role_resource_test.gointernal/provider/user_resource_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/provider/user_resource_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
internal/provider/role_resource.go (1)
internal/provider/role_schema.go (1)
RoleResourceModel(16-22)
🔇 Additional comments (9)
internal/provider/role_resource_test.go (1)
12-46: LGTM!The basic acceptance test covers create, read, import, and update flows correctly. The use of
acctest.RandomWithPrefixensures test isolation, and the test checks validate all expected attributes.internal/provider/role_resource.go (8)
20-31: LGTM!Interface compliance assertions and the resource struct follow Terraform Plugin Framework conventions.
37-66: LGTM!Schema is well-defined with appropriate plan modifiers and clear documentation explaining the required
descriptionfield due to Kinde API limitations.
68-83: LGTM!Standard provider configuration pattern with proper type assertion and error handling.
85-170: LGTM!Create implementation is solid: creates the role, optionally assigns permissions with deterministic ordering, and re-fetches to ensure state accuracy. Error handling is comprehensive at each step.
172-178: LGTM!Helper correctly creates a defensive copy before sorting.
180-226: LGTM!Read correctly handles out-of-band deletions by calling
RemoveResourceon 404, and properly sets permissions to null when empty. The defensive sorting ensures state consistency.
228-369: LGTM!Update method correctly diffs permissions between plan and state, removes obsolete ones individually, and adds new ones in batch. The re-fetch at the end ensures state accuracy. The sequential removal approach is acceptable since Terraform will reconcile any partial failures on the next apply.
371-389: LGTM!Delete is idempotent by silently succeeding on 404, which is the correct pattern for Terraform resources.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
internal/provider/connection_resource.go (2)
48-61: Consider simplifying the validation logic.The boolean expression at lines 55-56 is correct but could be more readable. The current logic checks if at least one field is set AND at least one is null, which correctly identifies the invalid state.
🔎 Alternative implementation
func (m *ConnectionOptionsModel) Validate() error { if m == nil { return nil } - // If either field is set, both must be set - if (!m.ClientID.IsNull() || !m.ClientSecret.IsNull()) && - (m.ClientID.IsNull() || m.ClientSecret.IsNull()) { + // Both fields must be set together or both null + if m.ClientID.IsNull() != m.ClientSecret.IsNull() { return fmt.Errorf("both client_id and client_secret must be set if either is provided") } return nil }
378-400: Consider validating options against strategy earlier.The
ValidateConfigmethod only validates options for OAuth2 strategies (line 388). If a user provides options for an unsupported strategy, the error won't surface until apply time inconvertOptionsToMap(line 374), rather than during plan/validation.🔎 Enhanced validation
func (r *ConnectionResource) ValidateConfig(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { var data ConnectionResourceModel resp.Diagnostics.Append(req.Config.Get(ctx, &data)...) if resp.Diagnostics.HasError() { return } // Validate strategy if !data.Strategy.IsNull() { strategy := connections.Strategy(data.Strategy.ValueString()) if strings.HasPrefix(string(strategy), "oauth2:") { // Validate options if present if data.Options != nil { if err := data.Options.Validate(); err != nil { resp.Diagnostics.AddError( "Invalid Options Configuration", err.Error(), ) } } + } else { + // Warn if options are provided for non-OAuth2 strategies + if data.Options != nil && !data.Options.IsEmpty() { + resp.Diagnostics.AddWarning( + "Options Ignored for Non-OAuth2 Strategy", + fmt.Sprintf("The strategy %q does not support options. The provided options will be ignored.", data.Strategy.ValueString()), + ) + } } } }internal/provider/user_resource.go (2)
159-173: Optional: Simplify redundant validation logic.Since
first_nameandlast_nameare validated as required on create (lines 159-173), the null checks at lines 312-324 will always evaluate to false (they will never be null after passing validation).Consider simplifying the Create method by removing the conditional null checks since these fields are guaranteed to be present:
🔎 Suggested simplification
- // Handle first_name: only set if it was in the plan - if !plan.FirstName.IsNull() { - plan.FirstName = types.StringValue(user.FirstName) - } else { - plan.FirstName = types.StringNull() - } - - // Handle last_name: only set if it was in the plan - if !plan.LastName.IsNull() { - plan.LastName = types.StringValue(user.LastName) - } else { - plan.LastName = types.StringNull() - } + // Set name fields (guaranteed to be present due to validation above) + plan.FirstName = types.StringValue(user.FirstName) + plan.LastName = types.StringValue(user.LastName)
785-844: Consider aligning ImportState behavior with Read for consistency.The
ImportStatemethod differs fromReadin how it handles optional fields:
Name fields (lines 839-840): ImportState always populates
first_nameandlast_name, while Read only sets them if previously tracked in state (lines 427-435). This means after import, these fields will persist in state even if not in user config.Suspension status (line 843): ImportState doesn't set
is_suspended, while Read preserves it if previously tracked (lines 437-440). This means imported users won't have their suspension status tracked unless users explicitly add it to config.While this may be intentional (name fields are commonly desired, suspension is opt-in), the inconsistency could confuse users about which fields are tracked post-import.
Consider:
- Either always populate all fields in ImportState to give users full visibility
- Or conditionally populate based on whether fields are in the importing config (though this is more complex)
- Document the import behavior clearly in resource documentation
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/resources/connection.mddocs/resources/user.mdinternal/provider/connection_resource.gointernal/provider/user_resource.go
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/resources/user.md
- docs/resources/connection.md
🔇 Additional comments (6)
internal/provider/connection_resource.go (6)
1-25: LGTM!The package declaration, imports, and resource setup follow Terraform provider best practices. Interface assertions ensure compile-time verification of required interfaces.
79-114: LGTM!The
optionsEmptyPreserveModifiercorrectly handles sensitive OAuth2 credentials that the API doesn't return. The fallback logic (config → state → null) ensures sensitive values are preserved during refresh operations.
120-177: LGTM!The schema properly addresses state encryption concerns with comprehensive warnings and practical examples. The
Sensitive: truemarkers provide CLI/UI redaction, and the plan modifier preserves values correctly.
179-194: LGTM!Standard provider configuration pattern with proper type checking and error handling.
196-334: LGTM!The CRUD lifecycle methods follow Terraform best practices. The Read method correctly preserves sensitive options from state since the API doesn't return them, and error handling is comprehensive throughout.
336-347: LGTM!The import implementation correctly handles the limitation that sensitive OAuth2 credentials cannot be retrieved from the API. The warning diagnostic properly informs users they must set these values in their configuration.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @internal/provider/role_resource_test.go:
- Around line 157-186: The helper function
testAccRoleResourceConfig_WithPermissions is unused and causing linter/CI
failures; remove its entire definition from
internal/provider/role_resource_test.go and ensure no tests reference it (tests
should continue using testAccRoleWithPermissionResourcesConfig); run `go
vet`/`golangci-lint` to confirm the unused symbol is gone.
🧹 Nitpick comments (5)
internal/provider/role_resource_test.go (2)
48-88: Track resolution of the permissions handling issue.The test structure looks correct now (both permissions are created in a single config), but it remains skipped due to a "Provider produced inconsistent result" error. This suggests an underlying issue in the role resource's permission handling logic.
Are you tracking this issue separately? The error message suggests the provider returns
nullfor permissions when a set with an empty string is expected. This might be related to how theReadmethod handles empty permission lists (see lines 207-217 inrole_resource.go).Would you like me to investigate the permission handling logic in
role_resource.goto identify potential causes?
90-135: Track resolution of the permissions handling issue.This test is also skipped due to the same "Provider produced inconsistent result" error. The incremental permission removal scenario would be valuable once the underlying permissions handling issue is resolved.
internal/provider/role_resource.go (3)
183-229: Consider usingflattenRoleResourcefor consistency.The
Readmethod constructs theRoleResourceModeldirectly (lines 219-225), whileCreateandImportStateuse theflattenRoleResourcehelper. This inconsistency makes the codebase harder to maintain. Additionally, the permission handling logic (lines 207-217) is duplicated in theUpdatemethod (lines 350-360).🔎 Proposed refactor to use flattenRoleResource consistently
func (r *RoleResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { var state RoleResourceModel diags := req.State.Get(ctx, &state) resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { return } role, err := r.client.Get(ctx, state.ID.ValueString()) if err != nil { if isNotFoundError(err) { resp.State.RemoveResource(ctx) return } resp.Diagnostics.AddError( "Error Reading Role", fmt.Sprintf("Could not read role ID %s: %s", state.ID.ValueString(), err), ) return } // Sort permissions without modifying original sortedPerms := sortPermissions(role.Permissions) - // If no permissions are returned, explicitly set to null - var permissionsSet types.Set - if len(sortedPerms) > 0 { - permissionsSet, diags = types.SetValueFrom(ctx, types.StringType, sortedPerms) - resp.Diagnostics.Append(diags...) - if resp.Diagnostics.HasError() { - return - } - } else { - permissionsSet = types.SetNull(types.StringType) + state, err = flattenRoleResource(ctx, role, sortedPerms) + if err != nil { + resp.Diagnostics.AddError( + "Error Setting Role State", + fmt.Sprintf("Could not set role state: %s", err), + ) + return } - - state = RoleResourceModel{ - ID: types.StringValue(role.ID), - Name: types.StringValue(role.Name), - Key: types.StringValue(role.Key), - Description: types.StringValue(role.Description), - Permissions: permissionsSet, - } diags = resp.State.Set(ctx, &state) resp.Diagnostics.Append(diags...) }
231-372: Consider usingflattenRoleResourcefor consistency and to eliminate duplication.Similar to
Read, this method:
- Constructs
RoleResourceModeldirectly instead of usingflattenRoleResource(lines 362-368)- Duplicates the permission handling logic from
Read(lines 350-360 are identical to lines 207-217 inRead)🔎 Proposed refactor
// Get the updated role to ensure we have all fields and permissions role, err := r.client.Get(ctx, plan.ID.ValueString()) if err != nil { resp.Diagnostics.AddError( "Error Reading Updated Role", fmt.Sprintf("Could not read updated role: %s", err), ) return } // Sort permissions without modifying original before setting state sortedPerms := sortPermissions(role.Permissions) - // If no permissions are returned, explicitly set to null - var permissionsSet types.Set - if len(sortedPerms) > 0 { - permissionsSet, diags = types.SetValueFrom(ctx, types.StringType, sortedPerms) - resp.Diagnostics.Append(diags...) - if resp.Diagnostics.HasError() { - return - } - } else { - permissionsSet = types.SetNull(types.StringType) + state, err = flattenRoleResource(ctx, role, sortedPerms) + if err != nil { + resp.Diagnostics.AddError( + "Error Setting Role State", + fmt.Sprintf("Could not set role state: %s", err), + ) + return } - - state = RoleResourceModel{ - ID: types.StringValue(role.ID), - Name: types.StringValue(role.Name), - Key: types.StringValue(role.Key), - Description: types.StringValue(role.Description), - Permissions: permissionsSet, - } diags = resp.State.Set(ctx, &state) resp.Diagnostics.Append(diags...) }
394-421: Consider consolidatingsortStringSliceandsortPermissionsfor consistency.
ImportStateusessortStringSlice(line 405) whileCreate,Read, andUpdateusesortPermissions. If both functions are functionally equivalent (both handle nil properly), consider usingsortPermissionsconsistently throughout the resource, or replace all calls withsortStringSlicefrom utils.This inconsistency was flagged in a previous review. While both functions now handle nil correctly, using a single function would improve maintainability.
🔎 Proposed fix
func (r *RoleResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { role, err := r.client.Get(ctx, req.ID) if err != nil { resp.Diagnostics.AddError( "Error Reading Kinde Role", "Could not read Kinde role ID "+req.ID+": "+err.Error(), ) return } // Sort the role's permissions for consistent ordering - sortedPermissions := sortStringSlice(role.Permissions) + sortedPermissions := sortPermissions(role.Permissions) state, err := flattenRoleResource(ctx, role, sortedPermissions) if err != nil { resp.Diagnostics.AddError( "Error Setting Role State", "Could not set role state: "+err.Error(), ) return } diags := resp.State.Set(ctx, &state) resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { return } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/provider/connection_resource.gointernal/provider/role_resource.gointernal/provider/role_resource_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
internal/provider/role_resource.go (1)
internal/provider/role_schema.go (1)
RoleResourceModel(16-22)
🪛 GitHub Actions: Tests
internal/provider/role_resource_test.go
[error] 157-157: golangci-lint: function 'testAccRoleResourceConfig_WithPermissions' is unused (unused)
🪛 GitHub Check: Build
internal/provider/role_resource_test.go
[failure] 157-157:
func testAccRoleResourceConfig_WithPermissions is unused (unused)
🔇 Additional comments (9)
internal/provider/connection_resource.go (6)
1-35: LGTM! Clean structure and proper type safety.The package imports, type assertions, constructor, and model definitions are well-structured. The type assertions ensure compile-time safety for the required Terraform interfaces, and the struct tags are correctly formatted for both the framework and JSON serialization.
37-77: LGTM! Validation and conversion logic is sound.The
ConnectionOptionsModelmethods are correctly implemented:
IsEmpty()properly handles both null and empty string casesValidate()correctly enforces the all-or-nothing constraint for OAuth2 credentialsToAPIOptions()safely converts to the SDK type with proper nil checks
79-114: LGTM! Plan modifier correctly handles non-returned sensitive values.The
optionsEmptyPreserveModifieris well-implemented to handle the scenario where the API doesn't return sensitive OAuth2 credentials. The logic correctly:
- Prefers explicit config values when present
- Preserves state values when config is removed (since API won't return them)
- Falls back to null when neither is present
This is the appropriate pattern for managing sensitive data that isn't returned by read operations.
116-194: LGTM! Schema is comprehensive with excellent security documentation.The schema definition is well-structured with:
- Proper attribute configurations and plan modifiers
- Comprehensive state encryption warning (lines 143-161) with concrete examples, addressing previous security concerns
- Appropriate
Sensitive: trueflags on OAuth2 credentials- Clean provider configuration with proper type assertions and error handling
196-334: LGTM! CRUD operations are correctly implemented.All resource lifecycle methods are properly implemented:
- Create: Validates options, converts to API format, stores plan including sensitive values
- Read: Fetches from API, updates non-sensitive fields, implicitly preserves sensitive options from state (since API doesn't return them)
- Update: Properly handles both plan and state, converts and sends options to API
- Delete: Straightforward cleanup with proper error handling
The pattern of preserving sensitive options in state while the API doesn't return them is correctly handled through the combination of the plan modifier and Read method behavior.
336-414: LGTM! Import, conversion, and validation are excellently implemented.The final methods are well-crafted:
ImportState (336-347): Properly imports the ID and includes a helpful warning that sensitive OAuth2 credentials must be set in configuration post-import—excellent UX.
convertOptionsToMap (349-376): Comprehensive switch covering all OAuth2 strategies with appropriate error handling for unsupported strategies.
ValidateConfig (378-414): Robust validation ensuring OAuth2 connections require options (with both credentials) and non-OAuth2 connections reject options. This catches configuration errors at plan time before API calls.
The validation logic works in concert with the conversion helper to ensure only valid configurations reach the API.
internal/provider/role_resource_test.go (1)
12-46: LGTM! Basic CRUD test coverage is solid.The test properly covers create, read, import, and update flows with appropriate attribute validations.
internal/provider/role_resource.go (2)
85-170: LGTM! Create method handles role and permission lifecycle correctly.The method properly:
- Creates the role with basic details
- Fetches complete role data
- Updates permissions with sorted ordering for determinism
- Re-fetches to ensure consistency
- Uses
flattenRoleResourcefor state mapping
374-392: LGTM! Delete method properly handles both success and not-found cases.The method correctly treats 404 errors as successful deletion, which is the recommended pattern for idempotent delete operations.
dtoxvanilla1991
force-pushed
the
move-repo
branch
from
8dc4978 to
1d2f2bd
Compare
3 participants
Explain your changes
This PR moves the Kinde Terraform Provider repository to the official Kinde organization and establishes the foundation for a comprehensive Terraform provider that covers the full Kinde Management API surface.
Current Implementation Status
✅ Implemented Resources & Data Sources
Resources:
kinde_api- API managementkinde_application- Application managementkinde_application_connection- Application-connection relationshipskinde_connection- Connection managementkinde_organization- Organization managementkinde_organization_user- Organization user managementkinde_permission- Permission managementkinde_role- Role managementkinde_user- User managementkinde_user_role- User role assignmentsData Sources:
kinde_api- API lookupkinde_application- Application lookupkinde_connections- Connection listing📋 Kinde Management API Coverage Analysis
The Kinde Management API provides 29 major endpoint categories with 200+ operations. Here's the current coverage:
✅ Fully/Partially Covered Categories (10/29)
❌ Missing Categories (19/29)
🔧 Technical Foundation
This provider is built using:
github.com/nxt-fwd/kinde-go)Current Coverage: 10/29 categories (~34% of API surface area)
Checklist
I have read the "Pull requests" section in the contributing guidelines.
I agree to the terms within the code of conduct.
🛟 If you need help, consider asking for advice over in the Kinde community.