Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

displaybody should respect new line [main] #2981

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

h0nIg
Copy link
Contributor

@h0nIg h0nIg commented Jul 2, 2024

Description of the Change

REQUEST and RESPONSE should behave the same with CF_TRACE=true and CF_TRACE=testfile. even across request logger.

fmt.Fprintf(display.ui.Out, "%s\n", RedactedValue)

_, err := logFile.WriteString(RedactedValue)

CLI stdout

Authenticating...
REQUEST: [2024-07-02T13:42:02Z]
POST /oauth/token HTTP/1.1
Host: login.xxxxx
Accept: application/json
Authorization: [PRIVATE DATA HIDDEN]
Connection: close
Content-Type: application/x-www-form-urlencoded
User-Agent: cf8/8.7.10+5b7ce3c.2024-04-04 (go1.22.1; amd64 linux)
[PRIVATE DATA HIDDEN]

RESPONSE: [2024-07-02T13:42:02Z]
HTTP/1.1 200 OK

CLI file output

REQUEST: [2024-07-02T11:14:36Z]
POST /oauth/token HTTP/1.1
Host: login.xxx
Accept: application/json
Authorization: [PRIVATE DATA HIDDEN]
Connection: close
Content-Type: application/x-www-form-urlencoded
User-Agent: cf8/8.7.10+5b7ce3c.2024-04-04 (go1.22.1; amd64 linux)
[PRIVATE DATA HIDDEN]
RESPONSE: [2024-07-02T11:14:36Z]
HTTP/1.1 200 OK

Why Is This PR Valuable?

proper parsing across all redaction code (uaa, cloud controler, ...)

return logger.output.DisplayMessage(fmt.Sprintf("[%s Content Hidden]", strings.Split(contentType, ";")[0]))

err = logger.output.DisplayBody(rawRequestBody)

uaa:

REQUEST: [2024-07-02T11:14:36Z]
POST /oauth/token HTTP/1.1
Host: login.xxx
Accept: application/json
Authorization: [PRIVATE DATA HIDDEN]
Connection: close
Content-Type: application/x-www-form-urlencoded
User-Agent: cf8/8.7.10+5b7ce3c.2024-04-04 (go1.22.1; amd64 linux)
[PRIVATE DATA HIDDEN]
RESPONSE: [2024-07-02T11:14:36Z]
HTTP/1.1 200 OK

cloud controller (including newline)

REQUEST: [2024-07-02T11:14:28Z]
GET / HTTP/1.1
Host: api.xxx
Accept: application/json
Content-Type: application/json
User-Agent: cf8/8.7.10+5b7ce3c.2024-04-04 (go1.22.1; amd64 linux)
[application/json Content Hidden]

RESPONSE: [2024-07-02T11:14:28Z]
HTTP/1.1 200 OK

How Urgent Is The Change?

Not urgent

@joaopapereira
Copy link
Contributor

Hey thanks for the PR do you mind rebasing your PR?

@h0nIg
Copy link
Contributor Author

h0nIg commented Jul 4, 2024

@joaopapereira done, unit tests green but still some flaky integration tests, any idea?


  [FAILED] Got stuck at:
      
  Waiting for:
      APP2-82d5acd4-30b8-4cc2-6f96-1835209e208b\s+e231607e-16bb-4314-74d0-034adc03bc47\s+create succeeded\s*\n
  In [It] at: /__w/cli/cli/integration/v7/isolated/service_command_test.go:179 @ 07/04/24 08:46:09.628

@h0nIg
Copy link
Contributor Author

h0nIg commented Jul 15, 2024

@joaopapereira @gururajsh may i ask to re-trigger the flapping "Tests / Integration tests / MIN CAPI / run-integration-tests"?

Do you see any show stoppers to get this merged?

@a-b
Copy link
Member

a-b commented Aug 16, 2024

@h0nIg, if you want to have this change released, please submit a pull request to the v8 branch too.

@h0nIg
Copy link
Contributor Author

h0nIg commented Aug 28, 2024

@a-b #3151 done

@a-b a-b changed the title displaybody should respect new line displaybody should respect new line [main] Oct 4, 2024
@h0nIg
Copy link
Contributor Author

h0nIg commented Oct 24, 2024

@a-b ping

@a-b
Copy link
Member

a-b commented Oct 24, 2024

Thanks for the heads up. We will merge as soon as the tests are green.

@h0nIg
Copy link
Contributor Author

h0nIg commented Oct 24, 2024

@a-b

The runner has received a shutdown signal. This can happen when the runner service is stopped, or a manually started runner is canceled.

@h0nIg
Copy link
Contributor Author

h0nIg commented Dec 13, 2024

@a-b the tests are green, may i ask for a merge please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants