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

Basic auth #3393

Merged
merged 8 commits into from
Nov 7, 2018
Merged

Basic auth #3393

merged 8 commits into from
Nov 7, 2018

Conversation

ycao56
Copy link
Contributor

@ycao56 ycao56 commented Oct 17, 2018

  1. enable basic auth for prob and app
  2. add flags
  3. env could be used to override. this is good for running on k8s and override using secret

@bboreham
Copy link
Collaborator

Thanks for the PR!

I'm interested you did this on the probe too - by default the probe doesn't listen, and isn't expected to have user conversations - it can be turned on for debug or instrumentation purposes. What was the motivation there?

@bboreham
Copy link
Collaborator

(also the CI build is failing - see https://circleci.com/gh/weaveworks/scope/10262)

@ycao56
Copy link
Contributor Author

ycao56 commented Oct 18, 2018

@bboreham Once basic auth is enabled for app, probe needs to have correct authorization header in the request when feeding the data to app. That's why these flags are introduced in probe as well.

@ycao56
Copy link
Contributor Author

ycao56 commented Oct 25, 2018

@bboreham is there anything I need to do to fix the build? The integration-tests is failing but it doesn't clear to me why it fails.

#!/bin/bash -eo pipefail
goveralls -repotoken $COVERALLS_REPO_TOKEN -coverprofile=profile.cov -service=circleci

exit status 2: warning: no packages being tested depend on github.com/weaveworks/scope/app/multitenant
warning: no packages being tested depend on github.com/weaveworks/scope/cri/runtime
warning: no packages being tested depend on github.com/weaveworks/scope/extras/copyreport
warning: no packages being tested depend on github.com/weaveworks/scope/extras/dialer/src
warning: no packages being tested depend on github.com/weaveworks/scope/extras/example/searchapp
warning: no packages being tested depend on github.com/weaveworks/scope/extras/example/shout
warning: no packages being tested depend on github.com/weaveworks/scope/extras/fixprobe
warning: no packages being tested depend on github.com/weaveworks/scope/probe/appclient
warning: no packages being tested depend on github.com/weaveworks/scope/probe/cri
warning: no packages being tested depend on github.com/weaveworks/scope/probe/plugins
warning: no packages being tested depend on github.com/weaveworks/scope/prog
warning: no packages being tested depend on github.com/weaveworks/scope/prog/externalui
warning: no packages being tested depend on github.com/weaveworks/scope/prog/staticui
warning: no packages being tested depend on github.com/weaveworks/scope/render/expected
warning: no packages being tested depend on github.com/weaveworks/scope/test
warning: no packages being tested depend on github.com/weaveworks/scope/test/fixture
warning: no packages being tested depend on github.com/weaveworks/scope/test/utils
warning: no packages being tested depend on github.com/weaveworks/scope/test/weave
warning: no packages being tested depend on github.com/weaveworks/scope/tools/cover
warning: no packages being tested depend on github.com/weaveworks/scope/tools/runner
warning: no packages being tested depend on github.com/weaveworks/scope/tools/socks
# github.com/weaveworks/scope/probe/docker
/tmp/go-build081714680/github.com/weaveworks/scope/probe/docker/_obj/container.go:328:17: undefined: namespaceIPAddresses
# github.com/weaveworks/scope/vendor/github.com/weaveworks/tcptracer-bpf/vendor/github.com/iovisor/gobpf/elf
In file included from vendor/github.com/weaveworks/tcptracer-bpf/vendor/github.com/iovisor/gobpf/elf/elf.go:52:0:
./include/bpf.h:12:30: fatal error: linux/bpf_common.h: No such file or directory
 #include <linux/bpf_common.h>
                              ^
compilation terminated.
# github.com/weaveworks/scope/vendor/github.com/google/gopacket/pcap
vendor/github.com/google/gopacket/pcap/pcap.go:22:18: fatal error: pcap.h: No such file or directory
 #include <pcap.h>
                  ^
compilation terminated.
FAIL	github.com/weaveworks/scope/app [build failed]

Exited with code 1

@bboreham
Copy link
Collaborator

The integration-tests is failing

It doesn't run the integration tests for 3rd-party branches, to protect the resources used to run them. This is the reason most steps have test -z "$SECRET_PASSWORD" || ... at the beginning.

The error is just an untidy consequence of not having that test everywhere.

@bboreham
Copy link
Collaborator

I've pushed your branch to the weaveworks repo, so it should run all tests at https://circleci.com/workflow-run/04ca4fc6-af11-4af2-9e8b-e1246b28d409

@bboreham
Copy link
Collaborator

@ycao56 one more question: should we use a library such as https://github.com/goji/httpauth instead of implementing basic auth inside Scope?

@ycao56
Copy link
Contributor Author

ycao56 commented Nov 1, 2018

@bboreham Thanks for the suggestion. Code updated.

@ycao56
Copy link
Contributor Author

ycao56 commented Nov 5, 2018

@bboreham can you push the branch again to make the build pass?

@bboreham
Copy link
Collaborator

bboreham commented Nov 6, 2018

Branch re-pushed.

Looks like the gvt metadata wasn't updated for the new vendor'd files - this is described at https://github.com/weaveworks/scope/blob/master/vendor/README.md

I can fix it up if you like.

@ycao56
Copy link
Contributor Author

ycao56 commented Nov 6, 2018

@bboreham Is this what you are asking for? https://github.com/weaveworks/scope/pull/3393/files#diff-f502f3b8ea2828482e72c368c0cfd23d
If not, feel free to fix it for me. Thanks for your help.

@bboreham
Copy link
Collaborator

bboreham commented Nov 7, 2018

Yes, sorry, no idea how I missed that.

@bboreham bboreham merged commit 62d5559 into weaveworks:master Nov 7, 2018
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.

2 participants