Allow space application supporter to access specific sidecar endpoints

[mkocher]

allows access to:
GET /v3/sidecars/:guid
GET /v3/processes/:process_guid/sidecars
GET /v3/apps/:app_guid/sidecars

Finishes #2231

Co-authored-by: Maria Shaldybin mariash@vmware.com

Thanks for contributing to cloud_controller_ng. To speed up the process of reviewing your pull request please provide us with:

• I have reviewed the contributing guide

• I have viewed, signed, and submitted the Contributor License Agreement

• I have made this pull request to the main branch

• I have run all the unit tests using bundle exec rake

• I have run CF Acceptance Tests

[sweinstein22]

Acceptance

Setup

Confirmed user only has space application supporter and organization user roles:

$ cf curl /v3/roles?user_guids="0776980f-9ea5-499a-9a23-ecc09d0d5b39"
{
  "pagination": {
    ...
  },
  "resources": [
    {
      "guid": "7555955f-5baf-4700-93e1-3952ef664a1f",
      "type": "organization_user",
      "relationships": {
        "user": {
          "data": {
            "guid": "0776980f-9ea5-499a-9a23-ecc09d0d5b39"
          }
        },
       ...
    },
    {
      "guid": "e158488c-2b95-4806-a6f1-68df9dde8fdc",
      "type": "space_application_supporter",
      "relationships": {
        "user": {
          "data": {
            "guid": "0776980f-9ea5-499a-9a23-ecc09d0d5b39"
          }
        },
      ...
    }
  ]
}

Pushed an app from the set of sample apps with sidecars

Behavior Check

Ran cf curl against /v3/sidecars/:guid, /v3/processes/:process_guid/sidecars, and /v3/apps/:app_guid/sidecars as both a space application supporter and space developer, piped output to files and checked for a diff. Also manually confirmed that output looked correct.

$ diff space-app-supporter-sidecar-guid space-dev-sidecar-guid

$ diff space-app-supporter-process-sidecars space-dev-process-sidecars

$ diff space-app-supporter-app-sidecars space-dev-app-sidecars

Code Check

I'm wondering if we should be collapsing down the context 'with a user' and context 'permissions' sections. Since the shared examples specs have a functionality for checking the response objects, and it allows us to check that the response is correct for each role, it feels like a good opportunity to condense some of the tests down. Plus it would more closely follow what's been done in previous space app supporter work. Let me know what y'all think, or if I'm missing a reason we've kept things separated, otherwise look good!

Docs

I'm noticing that we've left off the 'Experimental' indicator in the docs, was the thinking that sidecars are in the experimental section of the docs so we don't necessarily need to indicate that the role is also experimental? It feels a bit like we're conflating experimental indicators in leaving it off, since the rest of the roles aren't experimental.

[mkocher]

We didn't collapse down the spec sections but we changed the context/describes to be more clear on what cases they're testing. The shared behaviors aren't testing the response format so the other test is serving a purpose.

Updated the docs to include experimental.

[sweinstein22]

Following a conversation with folks, we've agreed:

• the shared examples spec should include a guid check for list endpoints, as they'll always return a 200 but the list may not be populated
• if the response body is different depending on the role, the shared example spec should include a full rundown of the response body for each role
• the shared examples spec for an individual resource doesn't necessarily need to include the guid check, as the error code will actually change based on permissions. This test would fail if someone throws an errant "if user has x role return a 200 without a response body", but that's a much less likely mistake to make
• In the case where we've split out a content test from the permissions test, we're going to ensure that the context description does not reference a specific role, as it may be misleading for future contributors. Instead we're going to lean towards indicating that the test is meant to check the return body content.

Once changes have been made per the above these changes should be good to go!

[sweinstein22]

Looks great, thanks for making the adjustments @galenhammond and @MerricdeLauney !