Spike: Perform validations when creating and updating an App

[tcdowney]

BLOCKED by #4

Background

Cloud Controller validates changes made to an app, both when a request comes in (message level validation), and when it persists the app in the database (model level validation). Our API shim will need to perform similar structural/lightweight semantic validation of the request and a validating admission webhook needs to exist to take the place of our backend validation to provide a consistent experience for both CF API and Kubernetes CR users.

What to do

• Experiment with creating/updating apps on CF for VMs and make note of what happens when you take invalid actions (use the name of an existing app, use a name with invalid characters, invalid environment variables, etc.)
• Think about what validations apply only to the CF API shim and what apply to both the shim and kubectl users
• Think about additional Kubernetes-specific validations we should consider... perhaps we can use the webhook to ensure our app guids are unique across namespaces
• Update the shim to validate the request and return similar error responses (422 with a message?)
• Spike out a validating admission controller to perform consistent backend validation of the App CR
• Note any interestings on this issue as a comment for future validations/expansion

Note: The purpose of this spike is to gain experience with CF API validations and to be able to provide a POC that we can refer to in future discussions / story planning. In practice it may not be necessary to reach full parity with the validations in Cloud Controller.

Acceptance

When I go to this repo's README and follow the instructions to deploy the validating admission webhook Then I am able to run through the following scenarios correctly

Scenarios

1. User submits invalid JSON to the CF API shim

Given I have the shim/controllers/webhook deployed When I create an App with an invalid payload

curl "https://api-shim.example.org/v3/apps" \
  -X POST \
  -H "Content-type: application/json" \
  -d '{"name": "my-app", "invalid": "param"}'

Then I see the following error response:

Code: 422 Body:

{
   "errors": [
      {
         "detail": "Unknown field(s): 'invalid', Relationships 'relationships' is not an object",
         "title": "CF-UnprocessableEntity",
         "code": 10008
      }
   ]
}

2. CF API Shim: User attempts to create an App with a name that already exists in the namespace

(we are assuming the Kubernetes name for the App CRD is actually its guid)

Given I have already created an App with the name my-app in the space (namespace) my-space-guid When I use the shim to create a new App with the name my-app

curl "https://api-shim.example.org/v3/apps" \
  -X POST \
  -H "Content-type: application/json" \
  -d '{
    "name": "my-app",
    "relationships": {
      "space": {
        "data": {
          "guid": "my-space-guid"
        }
      }
    }
  }'

Then I see the following error response:

Code: 422 Body:

{
  "errors": [
    {
      "detail": "App with the name 'my-app' already exists.",
      "title": "CF-UniquenessError",
      "code": 10016
    }
  ]
}

3. kubectl apply: User attempts to create an App with a name (spec.name) that already exists in the namespace

(we are assuming the Kubernetes name for the App CRD is actually its guid)

Given I have already created an App with the name my-app in the space (namespace) my-space-guid When I kubectl apply a new custom resource that has an app with Spec.Name = my-guid (but a different Metadata.Name) Then I get an error back from the Kubernetes API and the CR is not applied

Dev Notes

• You can view a sample CF App CR here: https://github.com/cloudfoundry/cf-crd-explorations/blob/main/config/samples/apps_v1alpha1_app.yaml

For CF Apps these validations currently take place in the following places:

• https://github.com/cloudfoundry/cloud_controller_ng/blob/main/app/messages/app_create_message.rb

• https://github.com/cloudfoundry/cloud_controller_ng/blob/aeba231e4005af5f4782db80020d7b69f0973134/app/models/runtime/app_model.rb#L72-L80

• Check out: https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#validatingadmissionwebhook

• https://kubernetes.io/blog/2019/03/21/a-guide-to-kubernetes-admission-controllers/

• https://book.kubebuilder.io/cronjob-tutorial/webhook-implementation.html

[heycait]

Notes on validation

App already exists

{
  "errors": [
    {
      "detail": "App with the name 'dora' already exists.",
      "title": "CF-UniquenessError",
      "code": 10016
    }
  ]
}

Invalid app name: Seems to accept various things like -_&. See CAPI code

App Name with leading "/" works

cf curl -X POST /v3/apps -d '{"name":"/dora4", "relationships": {"space": {"data": {"guid": "d4408a90-d626-41fb-b622-7b84ee06e158"}}}}' -v

{
  "name": "/dora4",
  "relationships": {
    "space": {
      "data": {
        "guid": "d4408a90-d626-41fb-b622-7b84ee06e158"
      }
    }
  }
}

App name with leading "\" does not work

cf curl -X POST /v3/apps -d '{"name":"\dora4", "relationships": {"space": {"data": {"guid": "d4408a90-d626-41fb-b622-7b84ee06e158"}}}}' -v
{
  "errors": [
    {
      "detail": "Request invalid due to parse error: invalid request body",
      "title": "CF-MessageParseError",
      "code": 1001
    }
  ]
}

App name with double leading "\" works

  cf curl -X POST /v3/apps -d '{"name":"\\dora4", "relationships": {"space": {"data": {"guid": "d4408a90-d626-41fb-b622-7b84ee06e158"}}}}' -v

Invalid env var

cf curl -X POST /v3/apps -d '{"environment_variables": "", ""name":"dora2", "relationships": {"space": {"data": {"guid": "d4408a90-d626-41fb-b622-7b84ee06e158"}}}}' -v

{
  "errors": [
    {
      "detail": "Request invalid due to parse error: invalid request body",
      "title": "CF-MessageParseError",
      "code": 1001
    }
  ]
}

Lifecycle Errors Not passing lifecycle stack defaults to cflinuxfs3

Empty buildpacks

cf curl -X POST /v3/apps -d '{"lifecycle": {"type":"buildpack", "data": {"buildpacks": ""}}, "name":"dora4", "relationships": {"space": {"data": {"guid": "d4408a90-d626-41fb-b622-7b84ee06e158"}}}}' -v

{
  "errors": [
    {
      "detail": "Lifecycle Buildpacks must be an array",
      "title": "CF-UnprocessableEntity",
      "code": 10008
    }
  ]
}

Invalid buildpack

cf curl /v3/apps/1ffb0610-e44a-4cb4-a5b3-0fc2e8d470e7 -X PATCH -d '{"lifecycle":{"type":"buildpack", "data":{"buildpacks":["idontexist"]}}}'
  {
     "errors": [
        {
           "detail": "Buildpack \"idontexist\" must be an existing admin buildpack or a valid git URI",
           "title": "CF-UnprocessableEntity",
           "code": 10008
        }
     ]
  }

missing data section

cf curl -X POST /v3/apps -d '{"lifecycle": {"type":"buildpack"}, "name":"dora4", "relationships": {"space": {"data": {"guid": "d4408a90-d626-41fb-b622-7b84ee06e158"}}}}' -v

{
  "errors": [
    {
      "detail": "Lifecycle data must be an object",
      "title": "CF-UnprocessableEntity",
      "code": 10008
    }
  ]
}

empty lifecycle

cf curl -X POST /v3/apps -d '{"lifecycle": {}, "name":"dora4", "relationships": {"space": {"data": {"guid": "d4408a90-d626-41fb-b622-7b84ee06e158"}}}}' -v
{
  "errors": [
    {
      "detail": "Lifecycle data must be an object",
      "title": "CF-UnprocessableEntity",
      "code": 10008
    }
  ]
}

Empty data works

cf curl -X POST /v3/apps -d '{"lifecycle": {"type":"buildpack", "data": {}}, "name":"dora4", "relationships": {"space": {"data": {"guid": "d4408a90-d626-41fb-b622-7b84ee06e158"}}}}' -v
  "lifecycle": {
    "type": "buildpack",
    "data": {
      "buildpacks": [

      ],
      "stack": "cflinuxfs3"
    }
  },

[Birdrock]

After some discussion, we need to think about our approach on scenario 3. Kubernetes cannot guarantee uniqueness because of its eventual consistency model. Concurrent creates could go through because the API is non-authoritative. Guaranteeing uniqueness would require one of the following:

• Optimistic locking with a standalone database and validation service.
• Periodic checking and synchronization of objects (remove contention)

[gnovv]

Working with @matt-royal

Re-openning this story due to an issue with the validating webhook.

Steps to reproduce:

When creating an app with the same spec.name as an app in a different namespace

k apply -f config/samples/cf-crds/apps_v1alpha1_app.yaml

Then create a temporary file with the following contents: (make sure the cf-workloads namespace exists!)

---
apiVersion: apps.cloudfoundry.org/v1alpha1
kind: App
metadata:
  name: my-app-guid-2
  labels:
    apps.cloudfoundry.org/appGuid: my-app-guid
    apps.cloudfoundry.org/appName: my-app-name
  namespace: cf-workloads
spec:
  name: my-app-name
  state: STARTED
  type: kpack
  lifecycle:
    data:
      buildpacks: []
      stack: cflinuxfs3
  envSecretName: my-app-guid-env
  currentDropletRef:
    kind: Droplet
    apiVersion: apps.cloudfoundry.org/v1alpha1
    name: kpack-droplet-guid

Expected Response:

app.apps.cloudfoundry.org/my-app-guid-2 created

Response Received:

Error from server: error when creating "config/samples/cf-crds/apps_v1alpha1_app.yaml": admission webhook "app-validation-webhook.default.svc" denied the request: App with the name already exists!

[akrishna90]

The bug above mentioned by @gnovv has been fixed with this MR