diff options
author | Krasimir Angelov <kangelov@gitlab.com> | 2019-10-07 06:12:02 +0300 |
---|---|---|
committer | Krasimir Angelov <kangelov@gitlab.com> | 2019-10-07 06:12:02 +0300 |
commit | f640b99697b6f6bb9c9ed2919bab3f1ee0d1c4e2 (patch) | |
tree | 0f81aae0a295097ac2eef9580bb72442449399d2 | |
parent | fa804e9f422cb1525324e917a4b318bea8596067 (diff) |
Remove unused Admin gRPC API
It was ever used. See
gitlab-org/omnibus-gitlab/merge_requests/3444#note_194217889 for more
details.
Done as part of https://gitlab.com/gitlab-org/gitlab/issues/28781.
-rw-r--r-- | .gitignore | 2 | ||||
-rw-r--r-- | admin_test.go | 220 | ||||
-rw-r--r-- | app.go | 58 | ||||
-rw-r--r-- | app_config.go | 19 | ||||
-rw-r--r-- | daemon.go | 2 | ||||
-rw-r--r-- | helpers.go | 23 | ||||
-rw-r--r-- | internal/admin/auth.go | 14 | ||||
-rw-r--r-- | internal/admin/server.go | 63 | ||||
-rw-r--r-- | main.go | 66 | ||||
-rw-r--r-- | testdata/.admin-secret | 1 |
10 files changed, 12 insertions, 456 deletions
@@ -3,8 +3,6 @@ shared/pages/.update /gitlab-pages /vendor -admin.socket - # Used by the makefile /.GOPATH /bin diff --git a/admin_test.go b/admin_test.go deleted file mode 100644 index ab82137b..00000000 --- a/admin_test.go +++ /dev/null @@ -1,220 +0,0 @@ -package main - -import ( - "context" - "crypto/tls" - "net" - "net/http" - "net/http/httptest" - "os" - "testing" - "time" - - "github.com/stretchr/testify/require" - gitalyauth "gitlab.com/gitlab-org/gitaly/auth" - "golang.org/x/net/http2" - "google.golang.org/grpc" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/credentials" - healthpb "google.golang.org/grpc/health/grpc_health_v1" - "google.golang.org/grpc/status" -) - -var ( - // Use ../../ because the pages binary interprets the path in ./shared/pages - adminSecretArgs = []string{"-admin-secret-path", "../../testdata/.admin-secret"} - adminToken = "super-secret\n" -) - -func TestAdminUnixPermissions(t *testing.T) { - skipUnlessEnabled(t) - socketPath := "admin.socket" - // Use "../../" because the pages executable cd's into shared/pages - adminArgs := append(adminSecretArgs, "-admin-unix-listener", "../../"+socketPath) - teardown := RunPagesProcessWithoutWait(t, *pagesBinary, listeners, "", adminArgs...) - defer teardown() - - waitHTTP2RoundTripUnix(t, socketPath) - - st, err := os.Stat(socketPath) - require.NoError(t, err) - expectedMode := os.FileMode(0777) - require.Equal(t, expectedMode, st.Mode()&expectedMode, "file permissions of unix socket") -} - -func TestAdminHealthCheckUnix(t *testing.T) { - skipUnlessEnabled(t) - socketPath := "admin.socket" - // Use "../../" because the pages executable cd's into shared/pages - adminArgs := append(adminSecretArgs, "-admin-unix-listener", "../../"+socketPath) - teardown := RunPagesProcessWithoutWait(t, *pagesBinary, listeners, "", adminArgs...) - defer teardown() - - waitHTTP2RoundTripUnix(t, socketPath) - - testCases := []struct { - desc string - dialOpt grpc.DialOption - code codes.Code - }{ - { - desc: "no auth provided", - code: codes.Unauthenticated, - }, - { - desc: "wrong auth provided", - dialOpt: grpc.WithPerRPCCredentials(gitalyauth.RPCCredentials("wrong token")), - code: codes.PermissionDenied, - }, - { - desc: "correct auth provided", - dialOpt: grpc.WithPerRPCCredentials(gitalyauth.RPCCredentials(adminToken)), - code: codes.OK, - }, - } - - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - connOpts := []grpc.DialOption{ - grpc.WithInsecure(), - grpcUnixDialOpt(), - } - if tc.dialOpt != nil { - connOpts = append(connOpts, tc.dialOpt) - } - - conn, err := grpc.Dial(socketPath, connOpts...) - require.NoError(t, err, "dial") - defer conn.Close() - - err = healthCheck(conn) - require.Equal(t, tc.code, status.Code(err), "wrong grpc code: %v", err) - }) - } -} - -func TestAdminHealthCheckHTTPS(t *testing.T) { - skipUnlessEnabled(t) - key, cert := CreateHTTPSFixtureFiles(t) - creds, err := credentials.NewClientTLSFromFile(cert, "") - require.NoError(t, err, "grpc client credentials") - - adminAddr := newAddr() - adminArgs := []string{"-admin-https-listener", adminAddr, "-admin-https-key", key, "-admin-https-cert", cert} - adminArgs = append(adminArgs, adminSecretArgs...) - - teardown := RunPagesProcessWithoutWait(t, *pagesBinary, listeners, "", adminArgs...) - defer teardown() - - waitHTTP2RoundTrip(t, adminAddr) - - testCases := []struct { - desc string - dialOpt grpc.DialOption - code codes.Code - }{ - { - desc: "no auth provided", - code: codes.Unauthenticated, - }, - { - desc: "wrong auth provided", - dialOpt: grpc.WithPerRPCCredentials(gitalyauth.RPCCredentials("wrong token")), - code: codes.PermissionDenied, - }, - { - desc: "correct auth provided", - dialOpt: grpc.WithPerRPCCredentials(gitalyauth.RPCCredentials(adminToken)), - code: codes.OK, - }, - } - - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - connOpts := []grpc.DialOption{ - grpc.WithTransportCredentials(creds), - } - if tc.dialOpt != nil { - connOpts = append(connOpts, tc.dialOpt) - } - conn, err := grpc.Dial(adminAddr, connOpts...) - require.NoError(t, err, "dial") - defer conn.Close() - - err = healthCheck(conn) - require.Equal(t, tc.code, status.Code(err), "wrong grpc code: %v", err) - }) - } -} - -func newAddr() string { - s := httptest.NewServer(http.NotFoundHandler()) - s.Close() - return s.Listener.Addr().String() -} - -func waitHTTP2RoundTrip(t *testing.T, addr string) { - transport := &http2.Transport{ - TLSClientConfig: &tls.Config{RootCAs: TestCertPool}, - } - - req, err := http.NewRequest("get", "https://"+addr, nil) - require.NoError(t, err) - - for start := time.Now(); time.Since(start) < 5*time.Second; time.Sleep(100 * time.Millisecond) { - var response *http.Response - response, err = transport.RoundTrip(req) - if err == nil { - response.Body.Close() - return - } - } - - t.Fatal(err) -} - -func grpcUnixDialOpt() grpc.DialOption { - return grpc.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) { - return net.DialTimeout("unix", addr, timeout) - }) -} - -func waitHTTP2RoundTripUnix(t *testing.T, socketPath string) { - var err error - - for start := time.Now(); time.Since(start) < 5*time.Second; time.Sleep(100 * time.Millisecond) { - err = roundtripHTTP2Unix(socketPath) - if err == nil { - return - } - } - - t.Fatal(err) -} - -func roundtripHTTP2Unix(socketPath string) error { - transport := &http2.Transport{ - DialTLS: func(network, addr string, cfg *tls.Config) (net.Conn, error) { - return net.Dial("unix", socketPath) - }, - } - req, err := http.NewRequest("get", "https://localhost/", nil) - if err != nil { - return err - } - - resp, err := transport.RoundTrip(req) - if err != nil { - return err - } - return resp.Body.Close() -} - -func healthCheck(conn *grpc.ClientConn) error { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - client := healthpb.NewHealthClient(conn) - _, err := client.Check(ctx, &healthpb.HealthCheckRequest{}) - return err -} @@ -3,10 +3,8 @@ package main import ( "crypto/tls" "errors" - "fmt" "net" "net/http" - "os" "sync" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -17,7 +15,6 @@ import ( mimedb "gitlab.com/lupine/go-mimedb" "gitlab.com/gitlab-org/gitlab-pages/internal/acme" - "gitlab.com/gitlab-org/gitlab-pages/internal/admin" "gitlab.com/gitlab-org/gitlab-pages/internal/artifact" "gitlab.com/gitlab-org/gitlab-pages/internal/auth" headerConfig "gitlab.com/gitlab-org/gitlab-pages/internal/config" @@ -351,9 +348,6 @@ func (a *theApp) Run() { a.listenMetricsFD(&wg, a.ListenMetrics) } - a.listenAdminUnix(&wg) - a.listenAdminHTTPS(&wg) - a.domains.Watch(a.Domain) wg.Wait() @@ -408,58 +402,6 @@ func (a *theApp) listenMetricsFD(wg *sync.WaitGroup, fd uintptr) { }() } -func (a *theApp) listenAdminUnix(wg *sync.WaitGroup) { - fd := a.ListenAdminUnix - if fd == 0 { - return - } - - wg.Add(1) - go func() { - defer wg.Done() - - l, err := net.FileListener(os.NewFile(fd, "[admin-socket-unix]")) - if err != nil { - errortracking.Capture(err, errortracking.WithField("listener", "admin unix socket")) - fatal(fmt.Errorf("failed to listen on FD %d: %v", fd, err)) - } - defer l.Close() - - if err := admin.NewServer(string(a.AdminToken)).Serve(l); err != nil { - fatal(err) - } - }() -} - -func (a *theApp) listenAdminHTTPS(wg *sync.WaitGroup) { - fd := a.ListenAdminHTTPS - if fd == 0 { - return - } - - cert, err := tls.X509KeyPair(a.AdminCertificate, a.AdminKey) - if err != nil { - capturingFatal(err) - } - - wg.Add(1) - go func() { - defer wg.Done() - - l, err := net.FileListener(os.NewFile(fd, "[admin-socket-https]")) - if err != nil { - errMsg := fmt.Errorf("failed to listen on FD %d: %v", fd, err) - log.WithError(errMsg).Error("error") - capturingFatal(err, errortracking.WithField("listener", "admin https socket")) - } - defer l.Close() - - if err := admin.NewTLSServer(string(a.AdminToken), &cert).Serve(l); err != nil { - fatal(err) - } - }() -} - func runApp(config appConfig) { a := theApp{appConfig: config, domains: source.NewDomains()} diff --git a/app_config.go b/app_config.go index 00469db6..b870626d 100644 --- a/app_config.go +++ b/app_config.go @@ -6,20 +6,15 @@ type appConfig struct { ArtifactsServerTimeout int RootCertificate []byte RootKey []byte - AdminCertificate []byte - AdminKey []byte - AdminToken []byte MaxConns int - ListenHTTP []uintptr - ListenHTTPS []uintptr - ListenProxy []uintptr - ListenMetrics uintptr - ListenAdminUnix uintptr - ListenAdminHTTPS uintptr - InsecureCiphers bool - TLSMinVersion uint16 - TLSMaxVersion uint16 + ListenHTTP []uintptr + ListenHTTPS []uintptr + ListenProxy []uintptr + ListenMetrics uintptr + InsecureCiphers bool + TLSMinVersion uint16 + TLSMaxVersion uint16 HTTP2 bool RedirectHTTP bool @@ -271,8 +271,6 @@ func updateFds(config *appConfig, cmd *exec.Cmd) { for _, fdPtr := range []*uintptr{ &config.ListenMetrics, - &config.ListenAdminUnix, - &config.ListenAdminHTTPS, } { if *fdPtr != 0 { *fdPtr = daemonUpdateFd(cmd, *fdPtr) @@ -27,29 +27,6 @@ func createSocket(addr string) (net.Listener, *os.File) { return l, fileForListener(l) } -// Be careful: if you let either of the return values get garbage -// collected by Go they will be closed automatically. -func createUnixSocket(addr string) (net.Listener, *os.File) { - if err := os.Remove(addr); err != nil && !os.IsNotExist(err) { - fatal(err) - } - - l, err := net.Listen("unix", addr) - if err != nil { - fatal(err) - } - - // This socket should be world-accessible; we have authentication at the - // application level. When pages runs with privilege separation, the - // default permissions will prevent gitlab-rails from connecting to the - // admin socket. - if err := os.Chmod(addr, 0777); err != nil { - fatal(err) - } - - return l, fileForListener(l) -} - func fileForListener(l net.Listener) *os.File { type filer interface { File() (*os.File, error) diff --git a/internal/admin/auth.go b/internal/admin/auth.go deleted file mode 100644 index b8eeb618..00000000 --- a/internal/admin/auth.go +++ /dev/null @@ -1,14 +0,0 @@ -package admin - -import ( - "time" - - gitalyauth "gitlab.com/gitlab-org/gitaly/auth" - context "golang.org/x/net/context" -) - -func authFunc(token string) func(context.Context) (context.Context, error) { - return func(ctx context.Context) (context.Context, error) { - return ctx, gitalyauth.CheckToken(ctx, token, time.Now()) - } -} diff --git a/internal/admin/server.go b/internal/admin/server.go deleted file mode 100644 index e7c7cfe6..00000000 --- a/internal/admin/server.go +++ /dev/null @@ -1,63 +0,0 @@ -package admin - -import ( - "crypto/tls" - - grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" - grpc_auth "github.com/grpc-ecosystem/go-grpc-middleware/auth" - grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" - grpc_recovery "github.com/grpc-ecosystem/go-grpc-middleware/recovery" - grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" - log "github.com/sirupsen/logrus" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials" - "google.golang.org/grpc/health" - healthpb "google.golang.org/grpc/health/grpc_health_v1" -) - -var logrusEntry *log.Entry - -func init() { - logger := log.StandardLogger() - - logrusEntry = log.NewEntry(logger) - grpc_logrus.ReplaceGrpcLogger(logrusEntry) -} - -// NewServer creates a new unencrypted gRPC server for the gitlab-pages admin API. -func NewServer(secret string) *grpc.Server { - grpcServer := grpc.NewServer(serverOpts(secret)...) - registerServices(grpcServer) - return grpcServer -} - -// NewTLSServer creates a new gRPC server with encryption for the gitlab-pages admin API. -func NewTLSServer(secret string, cert *tls.Certificate) *grpc.Server { - grpcServer := grpc.NewServer(append( - serverOpts(secret), - grpc.Creds(credentials.NewServerTLSFromCert(cert)), - )...) - registerServices(grpcServer) - return grpcServer -} - -func serverOpts(secret string) []grpc.ServerOption { - return []grpc.ServerOption{ - grpc.StreamInterceptor(grpc_middleware.ChainStreamServer( - grpc_prometheus.StreamServerInterceptor, - grpc_logrus.StreamServerInterceptor(logrusEntry), - grpc_auth.StreamServerInterceptor(authFunc(secret)), - grpc_recovery.StreamServerInterceptor(), - )), - grpc.UnaryInterceptor(grpc_middleware.ChainUnaryServer( - grpc_prometheus.UnaryServerInterceptor, - grpc_logrus.UnaryServerInterceptor(logrusEntry), - grpc_auth.UnaryServerInterceptor(authFunc(secret)), - grpc_recovery.UnaryServerInterceptor(), - )), - } -} - -func registerServices(g *grpc.Server) { - healthpb.RegisterHealthServer(g, health.NewServer()) -} @@ -48,11 +48,11 @@ var ( daemonInplaceChroot = flag.Bool("daemon-inplace-chroot", false, "Fall back to a non-bind-mount chroot of -pages-root when daemonizing") logFormat = flag.String("log-format", "text", "The log output format: 'text' or 'json'") logVerbose = flag.Bool("log-verbose", false, "Verbose logging") - adminSecretPath = flag.String("admin-secret-path", "", "Path to the file containing the admin secret token") - adminUnixListener = flag.String("admin-unix-listener", "", "The path for the admin API unix socket listener (optional)") - adminHTTPSListener = flag.String("admin-https-listener", "", "The listen address for the admin API HTTPS listener (optional)") - adminHTTPSCert = flag.String("admin-https-cert", "", "The path to the certificate file for the admin API (optional)") - adminHTTPSKey = flag.String("admin-https-key", "", "The path to the key file for the admin API (optional)") + _ = flag.String("admin-secret-path", "", "DEPRECATED") + _ = flag.String("admin-unix-listener", "", "DEPRECATED") + _ = flag.String("admin-https-listener", "", "DEPRECATED") + _ = flag.String("admin-https-cert", "", "DEPRECATED") + _ = flag.String("admin-https-key", "", "DEPRECATED") secret = flag.String("auth-secret", "", "Cookie store hash key, should be at least 32 bytes long.") gitLabAuthServer = flag.String("auth-server", "", "DEPRECATED, use gitlab-server instead. GitLab server, for example https://www.gitlab.com") gitLabServer = flag.String("gitlab-server", "", "GitLab server, for example https://www.gitlab.com") @@ -142,9 +142,6 @@ func configFromFlags() appConfig { }{ {&config.RootCertificate, *pagesRootCert}, {&config.RootKey, *pagesRootKey}, - {&config.AdminCertificate, *adminHTTPSCert}, - {&config.AdminKey, *adminHTTPSKey}, - {&config.AdminToken, *adminSecretPath}, } { if file.path != "" { *file.contents = readFile(file.path) @@ -235,11 +232,6 @@ func appMain() { } log.WithFields(log.Fields{ - "admin-https-cert": *adminHTTPSCert, - "admin-https-key": *adminHTTPSKey, - "admin-https-listener": *adminHTTPSListener, - "admin-unix-listener": *adminUnixListener, - "admin-secret-path": *adminSecretPath, "artifacts-server": *artifactsServer, "artifacts-server-timeout": *artifactsServerTimeout, "daemon-gid": *daemonGID, @@ -275,8 +267,6 @@ func appMain() { for _, cs := range [][]io.Closer{ createAppListeners(&config), createMetricsListener(&config), - createAdminUnixListener(&config), - createAdminHTTPSListener(&config), } { defer closeAll(cs) } @@ -360,52 +350,6 @@ func createMetricsListener(config *appConfig) []io.Closer { return []io.Closer{l, f} } -// createAdminUnixListener returns net.Listener and *os.File instances. The -// caller must ensure they don't get closed or garbage-collected (which -// implies closing) too soon. -func createAdminUnixListener(config *appConfig) []io.Closer { - unixPath := *adminUnixListener - if unixPath == "" { - return nil - } - - if *adminSecretPath == "" { - fatal(fmt.Errorf("missing admin secret token file")) - } - - l, f := createUnixSocket(unixPath) - config.ListenAdminUnix = f.Fd() - - log.WithFields(log.Fields{ - "listener": unixPath, - }).Debug("Set up admin unix socket") - - return []io.Closer{l, f} -} - -// createAdminHTTPSListener returns net.Listener and *os.File instances. The -// caller must ensure they don't get closed or garbage-collected (which -// implies closing) too soon. -func createAdminHTTPSListener(config *appConfig) []io.Closer { - addr := *adminHTTPSListener - if addr == "" { - return nil - } - - if *adminSecretPath == "" { - fatal(fmt.Errorf("missing admin secret token file")) - } - - l, f := createSocket(addr) - config.ListenAdminHTTPS = f.Fd() - - log.WithFields(log.Fields{ - "listener": addr, - }).Debug("Set up admin HTTPS socket") - - return []io.Closer{l, f} -} - func printVersion(showVersion bool, version string) { if showVersion { fmt.Fprintf(os.Stdout, "%s\n", version) diff --git a/testdata/.admin-secret b/testdata/.admin-secret deleted file mode 100644 index afed63c2..00000000 --- a/testdata/.admin-secret +++ /dev/null @@ -1 +0,0 @@ -super-secret |