Conversation
Moved RawScrubber generator function to plans/util.go
|
|
||
| scrubber, err := plans.RawScrubber(spec.Config.Scrub) | ||
| if err != nil { | ||
| err = errors.Wrap(err, "spec for docker.logs has invalid scrubber spec") |
There was a problem hiding this comment.
I think for errors.Wrap we usually use a present-tense description of the action, e.g. "create scrubber" or "create scrubber for spec %s"
I don't have a good "why", seems to be the convention
There was a problem hiding this comment.
To be completely honest, I don't get what a present tense version of that message would be
There was a problem hiding this comment.
the errors occurred when trying to create the scrubber by calling plans.RawScrubber, so I think something like create scrubber would be fine.
There was a problem hiding this comment.
The entire point for that error message in particular is that it includes 'docker.logs' though, otherwise it's rather hard to figure out which of your 50 specs is causing the issue
dexhorthy
left a comment
dexhorthy
left a comment
There was a problem hiding this comment.
One thing about wrap, but probably meh-ish on that even. 
| line := lineScanner.Bytes() | ||
| line = task.RawScrubber(line) | ||
|
|
||
| // TODO: actually deal with errors besides 'buffer full' |
| scrubbedReader, scrubbedWriter := io.Pipe() | ||
| go func(readFrom io.Reader, writeTo *io.PipeWriter) { | ||
| lineScanner := bufio.NewScanner(readFrom) | ||
| byteWriter := bufio.NewWriter(writeTo) |
| } | ||
|
|
||
| if task.RawScrubber != nil { | ||
| scrubbedReader, scrubbedWriter := io.Pipe() |
There was a problem hiding this comment.
there might be enough duplicate code here to warrant factoring it out
There was a problem hiding this comment.
yeah +1 if it makes sense
…ort-bundle-option-to-scrub-of-sensitive
…ort-bundle-option-to-scrub-of-sensitive
|
There's evidently some issues with streams not getting passed properly - something for Monday. |
laverya
force-pushed
the
laverya/ch2742/support-bundle-option-to-scrub-of-sensitive
branch
4 times, most recently
from
77f4448 to
793d533
Compare
| args: ["fileThatDoesNotExist"] | ||
| container_id: `+containerID) | ||
|
|
||
| err := cmd.Generate( |
There was a problem hiding this comment.
small optimization -- If you are using the tmpdir stuff, you can just use GenerateBundle() helper if you want
There was a problem hiding this comment.
oh looks like maybe these tests are duplicated?
| line = scrubber(line) | ||
|
|
||
| // TODO: actually deal with errors besides 'buffer full' | ||
| n, _ := writeTo.Write(line) |
There was a problem hiding this comment.
can you check the error here and do writeTo.CloseWithError if it's non-nil?
There was a problem hiding this comment.
Right - no longer using buffio, so I no longer have to have that loop there
And errors are actually significant instead of normal
| for { | ||
| header, err := tr.Next() | ||
| if err == io.EOF { | ||
| Expect(err).NotTo(HaveOccurred(), "Failed to find "+targetFile+" in support bundle.") |
| return container.ID | ||
| } | ||
|
|
||
| // RemoveDockerContainer removes a docker container by name as cleanup. |
There was a problem hiding this comment.
super tiny -- this says remove by name but the param is ID
laverya
force-pushed
the
laverya/ch2742/support-bundle-option-to-scrub-of-sensitive
branch
2 times, most recently
from
414df43 to
68b56cd
Compare
stream-source and streams-source now properly apply filter regexes Integration tests added for: Copy file from container Run command on container Run command on container (stderr) Filter file read from container Filter output of command run on container
laverya
force-pushed
the
laverya/ch2742/support-bundle-option-to-scrub-of-sensitive
branch
from
3cffaee to
ddb2965
Compare
laverya
deleted the
laverya/ch2742/support-bundle-option-to-scrub-of-sensitive
branch
3 participants
Moved RawScrubber generator function to plans/util.go