Creating a space with asgs specified fails

[rahearn]

I'm trying to create a space with an existing running security group, but I'm getting an authorization error even though the user running the terraform is an OrgManager.

I'm using cloudfoundry provider version 0.15.3.

The relevant terraform code:

data "cloudfoundry_asg" "public" {
  name = "public_networks_egress"
}

resource "cloudfoundry_space" "public_egress" {
  name = "${local.cf_space_name}-egress"
  org = data.cloudfoundry_org.org.id
  asgs = [data.cloudfoundry_asg.public.id]
}

The output:

Terraform will perform the following actions:

  # cloudfoundry_space.public_egress will be created
  + resource "cloudfoundry_space" "public_egress" {
      + allow_ssh  = (known after apply)
      + asgs       = [
          + "<redacted GUID>",
        ]
      + auditors   = (known after apply)
      + developers = (known after apply)
      + id         = (known after apply)
      + managers   = (known after apply)
      + name       = "sandbox-egress"
      + org        = "<redacted GUID>"
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

cloudfoundry_space.public_egress: Creating...
╷
│ Error: You are not authorized to perform the requested action
│ 
│   with cloudfoundry_space.public_egress,
│   on main.tf line 29, in resource "cloudfoundry_space" "public_egress":
│   29: resource "cloudfoundry_space" "public_egress" {
│ 
╵

Creating the same space without asgs specified works fine, and then calling cf bind-security-group for that space also works.

[mogul]

Still present as of 0.50.2

[mogul]

Noting here that for a similar previous issue (OrgManagers and SpaceManagers couldn't set user roles), there was a PR that fixed the problem by falling back to a slower check using available perms when the admin permission isn't present. Something like that is probably needed here as well.

[damzog]

Hi @mogul,
the reason for this is that binding an asg to a space requires space developer privileges. But due to the fact that space role assignments to users is deferred and carved out into a separate resource of type cloudfoundry_space_users there is a chicken egg problem: You can't create the space because your current user lacks the authorization and you cant create cloudfoundry_space_users resource for your space until the space has already been created.

So I guess the onyl way out is the introduces a separate resource cloudfoundry_space_asgs

[mogul]

So I guess the onyl way out is the introduces a separate resource cloudfoundry_space_asgs

That makes sense, considering that if you're using the CLI, you're able to manipulate ASGs separate from the act of creating a space.

See also my issue about managing bindings separate from app creation. ;)

[damzog]

Actually I found a way which might not look so nice at first but I think it makes sense to actually add the user used by tf as developer or manager to a newly created space, see my pull request.

I guess you can still use the cf_space_users resource for all other users if you want.

If you insist you could rename the space attribute developers to tf-user or similar.

[mogul]

I think it makes sense to actually add the user used by tf as developer or manager to a newly created space

This actually matches the behavior of the CLI:

$ cf create-space test
Creating space test in org [myorg] as [myaccount]...
OK

Assigning role SpaceManager to user [myaccount] in org [myorg] / space test as [myaccount]...
OK

Assigning role SpaceDeveloper to user [myaccount] in org [myorg] / space test as [myaccount]...
OK

[mogul]

(So I think it should actually be implicit!)

[damzog]

@mogul
Yes, got your point. The identity terraform uses (lets call it tf-id) should have full authorization of all object tf manages.

Ok. I have changed my implementation. Would be great if you can have a look and merge it.

Do you want to create a bugfix branch?

[mogul]

I would but I'm not a maintainer! @ArthurHlt or @loafoe can you help?

[damzog]

There is one disputable aspect of the current implementation: The terraform user (tf-id) is added at create time. If (and only if) you add the same tf-id as part of the cf-space-user resource the create will work but on destroy once the f-space-user resource is destroyed any subsequent destroy of other space resources will fail.

Unfortunately I have no easy fix for this. You could authorize the tf-id explicitly in every space related resource at least at destroy time. I do not like this so I kept this simple implementation that requires the tf-id left out of the cf-space-user config.

[mogul]

I think that is reasonable behavior; people can just use depends_on to indicate dependencies between those resources.

[damzog]

@loafoe I have corrected the spelling error.

[damzog]

Hi @loafoe,

not sure if you have seen my comment on the pull request. So I repeat it here:

Regarding the permadiff : Default behaviour of the space_users resource is to ignore users outside the configured users. You can override this with the force flag, see also code . So there will be no permadiff. Also be aware that a space and space-users resource can be managed in one "tf script" only. Others would need to refer to it as data.

Regarding "a better fix would be to ensure the session user has sufficient permissions": Well the cf authorization model is rather simple. The authorization required for binding a sec-group to a space is space-developer or platform admin. However as platform operator I do not hand out platform admin privileges to my users. So this not an option. By the way this might need to be added to the testing strategy: In the README you suggest to test with an admin user in a pcf-dev environment. You should also test with an org manager only because that is the max privilige we will hand out to platform users.

Generally I think the root cause of the issue is the fact that space user management has been detached from the space resource to the space-users resource. Hence there is no way of authorizing the session user on a newly created space. It can be fixed by detaching all relations of a space into separate objects (actually there are only two asgs and isolation segments). Then one would need to make sure that the space-user resource is created first be making the other object depending on it.

Regarding "I also haven't seen this mechanism being used anywhere else in the provider": I just checked: The only resource where this could happen is the org itself. However the org has no attribute that requires a second api call after creation of the org itself. Hence the problem is not there.

[damzog]

Anyway this is clearly a bug. If you do not like the fix I provided than please state how this bug shall be fixed. For me it is a essential that an org manager can create spaces and assign asgs to it.

[damzog]

@ArthurHlt or @loafoe any update?

[damzog]

Actually I have created now another fix by introducing a separate resource space_asgs similar to the existing space_users. It works fine but I have trouble to create and run unit tests because I do not have pcf-dev or similar. Will try with bbl on gcp but this might take a while.

[mogul]

Same here... It's hard to contribute without a "spare" Cloud Foundry hanging around where we can run the tests! (I investigated using Korifi, which is pretty easy to set up locally, but haven't gotten to the point of actually running any tests yet.)

[loafoe]

@damzog thanks for staying on top of this. I saw your comments but was just swamped with other stuff last couple of days. Same as @mogul I don't have full access to a Cloud Foundry instance to test certain conditions. It's probably testament to how involved it is to set up a fully functional CF instance these days. I'll do a quick review and probably just merge your proposed solution as I don't see any other solution.

[damzog]

@loafoe Now it is done. I have filed #498 which should ultimately solve the issue. I have tested the provider with my current setup. Also I created a test case which I ran selectively against our development environment with success. I was not able to run all tests because of general constraints (users could not be created because of weak passwords making most test cases fail). But as the change is completely additive I think that is acceptable.

[damzog]

@loafoe any updates?

[mogul]

@loafoe sorry to keep pinging... Is there anyone else who could merge this?

[loafoe]

@damzog @mogul currently on holiday without physical access to the release signing keys. Back next week and will cut a release then.

[mogul]

Great, thanks so much!

[mogul]

Hey @loafoe are you back from holiday yet? If not, can you let us know how long you'll be out? If so, can you let us know when this might get merged into a release?

[loafoe]

Hey @loafoe are you back from holiday yet? If not, can you let us know how long you'll be out? If so, can you let us know when this might get merged into a release?

Hi, I actually cut a release yesterday which should include this change.

[mogul]

Oh I missed that... Thanks so much! I'll give this a try today.

[rahearn]

I tested out the changes in #498, but they don't work. Filed #575 about it.