rework-space-com / terraform-provider-freeipa

FreeIPA Terraform Provider
https://registry.terraform.io/providers/rework-space-com/freeipa/latest
GNU General Public License v3.0
22 stars 11 forks source link

Cannot create memberships in SUDO rules if the name contains slashes (/) #35

Closed FakeEmperor closed 3 weeks ago

FakeEmperor commented 1 month ago

SUDO rules with slashes in their names result in errors when managing them through the provider: adding any memberships (allowcmd, user, host, etc.) results in an error:

│ When applying changes to <RESOURCE>, provider
│ "provider[\"registry.terraform.io/rework-space-com/freeipa\"]" produced an unexpected new value: Root object was
│ present, but now absent.

For now I've made a note for our team to not use slashes (/) in any rules and made assertions in our modules, but the error is not intuitive and may lead some other provider users to spend time debugging.

Taking freeipa_sudo_rule_allowcmd_membership as an example, my understanding is that the ID for this resource (and similarly for others) is being set as such:

id := fmt.Sprintf("%s/%s/%s", d.Get("name").(string), cmd_id, d.Get("sudocmd").(string))

Which is parsed incorrectly later here if the / is present in d.Get("name"):

idParts := strings.SplitN(id, "/", 3)

I might be wrong, but this is my best guess.

I think there are a few options to handle it:

I presume same issue affects other resources with memberships:

But let me thank you for the amazing work you've done! ❤

Terraform Version, Provider Version and FreeIPA version

Terraform version: 1.9.3
FreeIPA provider version: 4.3.0
FreeIPA version: 4.12.1

Affected Resource(s)

If this issue appears to affect multiple resources, it may be an issue with Terraform's core, so please mention this. -->

Terraform Configuration Files

The following code fails:

resource "freeipa_sudo_cmd" "test_cmd" {
  name = "/usr/bin/tail /var/log/*"
}

resource "freeipa_user" "test_user" {
  name = "test"
  first_name = "Test"
  last_name = "Test"
}

resource "freeipa_sudo_rule" "test_rule" {
  name        = "test_rule_with/slash"
  enabled     = true
  description = "Rule name"
}

resource "freeipa_sudo_rule_allowcmd_membership" "test_rule" {
  name     = freeipa_sudo_rule.test_rule.name
  sudocmd  = freeipa_sudo_cmd.test_cmd
}

resource "freeipa_sudo_rule_user_membership" "test_rule" {
  name       = freeipa_sudo_rule.test_rule.name
  user       = freeipa_user.test_user.name
}

If you remove / in freeipa_sudo_rule.test_rule.name, it works. Notice that the name of the sudocmd does not matter.

Steps to Reproduce

  1. terraform plan to see the changes.
  2. terraform apply to observe the attempt to create memberships and failing with the aforementioned error.
  3. Remove the slash from `` and try again.
  4. Observe normal behavior.

Expected Behavior

Membership resources would be created normally.

Actual Behavior

Resources failed to be created if the rule name contains slashes with the following error:

╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to freeipa_sudo_rule_allowcmd_membership.test_rule, provider
│ "provider[\"registry.terraform.io/rework-space-com/freeipa\"]" produced an unexpected new value: Root object was
│ present, but now absent.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵
╷
│ Error: Provider produced inconsistent result after apply
│
│ When applying changes to freeipa_sudo_rule_user_membership.test_rule, provider
│ "provider[\"registry.terraform.io/rework-space-com/freeipa\"]" produced an unexpected new value: Root object was
│ present, but now absent.
│
│ This is a bug in the provider, which should be reported in the provider's own issue tracker.
╵

Community Note

infra-monkey commented 1 month ago

Indeed I can reproduce. your analysis is quite correct, any field that would be reused to generate the id of the resource will generate a conflict if it contains '/' I'll work on a fix, hopefully not restricting the name.

@FakeEmperor will you be available to test for non-regressions when it is ready?

infra-monkey commented 1 month ago

FYI: User groups and Host groups are not affected by this bug. However these type of resources are affected:

@FakeEmperor would you be kind to test this branch before I make the PR?

FakeEmperor commented 1 month ago

Sure! I will test it and get back to you. Thank you for the quick reply!