[Bug]: CLI does not appear to be sending the `Authorization` header to logging endpoints

tcdowney commented 2 years ago

What happened?

We recently introduced an AuthenticationMiddleware that validates that clients are sending valid credentials for the cluster in an Authorization header with each request. After this change we started seeing that CLI requests to the log-cache endpoints were failing with 401 errors.

This CLI output looks like this:

11:19 $ cf push node2
Pushing app node2 to org o / space s as tdowney@vmware.com...
Applying manifest file /Users/tdowney/go/src/github.com/cloudfoundry/cf-acceptance-tests/assets/node/manifest.yml...

Updating with these attributes...
  applications:
  - name: node2
    default-route: true
    env:
      PORT: 8080
Manifest applied
Packaging files to upload...
Uploading files...
 623 B / 623 B [======================================================================================================================================================================================] 100.00% 2s

Waiting for API to complete processing files...

Staging app and tracing logs...
Failed to retrieve logs from Log Cache: unexpected status code 401
Failed to retrieve logs from Log Cache: unexpected status code 401
Failed to retrieve logs from Log Cache: unexpected status code 401
Failed to retrieve logs from Log Cache: unexpected status code 401
Failed to retrieve logs from Log Cache: unexpected status code 401

Waiting for app node2 to start...

What you expected to happen

We don't support logging yet, so I expected it to only say "Staging app and tracing logs..." during this section:

Staging app and tracing logs...
Failed to retrieve logs from Log Cache: unexpected status code 401
Failed to retrieve logs from Log Cache: unexpected status code 401
Failed to retrieve logs from Log Cache: unexpected status code 401
Failed to retrieve logs from Log Cache: unexpected status code 401
Failed to retrieve logs from Log Cache: unexpected status code 401

Acceptance Criteria

GIVEN I have deployed cf-k8s-controllers (with the /api/v1/read/:guid log-cache endpoint protected by the AuthenticationMiddleware use a version of the code from before #364) WHEN I cf push an app THEN I see that the CF CLI sends a valid Authorization header to the API and that we don't get Failed to retrieve logs from Log Cache: unexpected status code 401 errors during push

How to reproduce it (as minimally and precisely as possible)

Clone out the master branch of the CF CLI (I used de37cbef85ee1c8464d5ba626f00c9d2b8309179)
Run make build
Add the built binary (in out) to your PATH
cf push an app and see 401 errors during the staging portion

Anything else we need to know?

In #364 we omitted the log-cache endpoints from the AuthenticationMiddleware as a temporary workaround. We'll need to undo part of this to be able to see the 401 errors again.

I started a Slack thread about this here: https://cloudfoundry.slack.com/archives/C0297673ASK/p1639505811233100

One potential area to look into is how the CLI is passing the AccessToken to the log-cache client versus how it does it for the CF API client: https://github.com/cloudfoundry/cli/blob/8c20e5118d7cb38f52d52b69060f6b77943dce6e/command/log_cache_client.go#L126

Environment

Revision of codebase:

95e7a474d0474b2d8b64a73bfe267f45ffb80ab7 of cf-k8s-controllers

de37cbef85ee1c8464d5ba626f00c9d2b8309179 of CF CLI Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"20", GitVersion:"v1.20.2", GitCommit:"faecb196815e248d3ecfb03c680a4507229c2a56", GitTreeState:"clean", BuildDate:"2021-01-14T05:14:17Z", GoVersion:"go1.15.6", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.3-gke.700", GitCommit:"e21de5d6cf1b351bc99285c3c762098b8617b773", GitTreeState:"clean", BuildDate:"2021-11-05T09:25:32Z", GoVersion:"go1.16.7b7", Compiler:"gc", Platform:"linux/amd64"}

Cloud provider or hardware configuration: GKE Others:

Dev Notes

https://github.com/cloudfoundry/cf-k8s-controllers/issues/362 made it so that the endpoint is unauthenticated right now. We'll want to undo that while working on this story to get the same error responses as the A/C.
Make sure we use the same CF for VMs / CF on K8s switch that the other auth code we added uses so we don't inadvertently break the CF for VMs flow

gcapizzi commented 2 years ago

The problem here is that the CLI uses a completely different client for Log Cache, so none of the things we did to make the Cloud Controller client work apply here. We could fix this, but is it correct to assume that the Log Cache endpoints will always share the same authentication with the CC ones?

tcdowney commented 2 years ago

We could fix this, but is it correct to assume that the Log Cache endpoints will always share the same authentication with the CC ones?

Yeah, they should. The flow in CF for VMs is this:

CLI sends auth to log-cache's cf-auth-proxy. The cf-auth-proxy hits an internal Cloud Controller API (/internal/v4/log_access/:guid / code) and based on the result of that endpoint decides whether or not to grant the CLI access to the logs. We'll need to tweak that flow a bit, but in general need to do something similar based on the user's access to a CFApp (or us giving them other access on their roles).

cc @acosta11 who's thinking about logging and metrics

a-b commented 2 years ago

Here is log-cache client on the CLI end. Let us know if you need any assistance.

matt-royal commented 2 years ago

There's WIP for this here: https://github.com/cloudfoundry/cli/compare/master...eirini-forks:pr-support-k8s-for-logging

Unfortunately, the existing tests for the k8s auth integration were failing and, once we addressed that, we found that the existing test for the Authorization header isn't putting the k8s config into a valid state. See this conversation thread where I ask for help from the EMEA team: https://cloudfoundry.slack.com/archives/C0297673ASK/p1646871220202859

Once this issue has been solved we can unblock this story.

matt-royal commented 2 years ago

@davewalter and I submitted a PR (https://github.com/cloudfoundry/cli/pull/2267) to add the auth header for logcache requests. We're waiting for feedback from the CLI team and from the EMEA folks.

julian-hj commented 2 years ago

Waiting for a release of the CLI to close this

tcdowney commented 2 years ago

CLI version 8.4.0 has these changes. https://github.com/cloudfoundry/cli/releases/tag/v8.4.0

cloudfoundry / korifi