Skip to content

Commit

Permalink
Migrate to Go errors from xerrors (#187)
Browse files Browse the repository at this point in the history
  • Loading branch information
int128 authored Feb 24, 2021
1 parent 0cb4379 commit 4be070c
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 58 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ require (
github.com/spf13/cobra v1.1.3
github.com/spf13/pflag v1.0.5
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1
k8s.io/api v0.20.4
k8s.io/apimachinery v0.20.4
k8s.io/cli-runtime v0.20.4
Expand Down
33 changes: 17 additions & 16 deletions pkg/authproxy/auth_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package authproxy

import (
"context"
"errors"
"fmt"
"net/url"
"strings"
"sync"
Expand All @@ -16,7 +18,6 @@ import (
"github.com/int128/kauthproxy/pkg/reverseproxy"
"github.com/int128/kauthproxy/pkg/transport"
"golang.org/x/sync/errgroup"
"golang.org/x/xerrors"
"k8s.io/api/core/v1"
"k8s.io/client-go/rest"
)
Expand All @@ -30,7 +31,7 @@ type Interface interface {
Do(ctx context.Context, in Option) error
}

var portForwarderConnectionLostError = xerrors.New("connection lost")
var portForwarderConnectionLostError = errors.New("connection lost")

// AuthProxy provides a use-case of authentication proxy.
type AuthProxy struct {
Expand Down Expand Up @@ -60,20 +61,20 @@ type Option struct {
func (u *AuthProxy) Do(ctx context.Context, o Option) error {
rsv, err := u.ResolverFactory.New(o.Config)
if err != nil {
return xerrors.Errorf("could not create a resolver: %w", err)
return fmt.Errorf("could not create a resolver: %w", err)
}
pod, containerPort, err := parseTargetURL(ctx, rsv, o.Namespace, o.TargetURL)
if err != nil {
return xerrors.Errorf("could not find the pod and container port: %w", err)
return fmt.Errorf("could not find the pod and container port: %w", err)
}
u.Logger.V(1).Infof("found container port %d of pod %s", containerPort, pod.Name)
transitPort, err := u.Env.AllocateLocalPort()
if err != nil {
return xerrors.Errorf("could not allocate a local port: %w", err)
return fmt.Errorf("could not allocate a local port: %w", err)
}
rpTransport, err := u.NewTransport(o.Config)
if err != nil {
return xerrors.Errorf("could not create a transport for reverse proxy: %w", err)
return fmt.Errorf("could not create a transport for reverse proxy: %w", err)
}
u.Logger.V(1).Infof("client -> reverse_proxy -> port_forwarder:%d -> pod -> container:%d", transitPort, containerPort)

Expand All @@ -99,15 +100,15 @@ func (u *AuthProxy) Do(ctx context.Context, o Option) error {
b := backoff.NewExponentialBackOff()
if err := backoff.Retry(func() error {
if err := u.run(ctx, ro); err != nil {
if xerrors.Is(err, portForwarderConnectionLostError) {
if errors.Is(err, portForwarderConnectionLostError) {
u.Logger.Printf("retrying: %s", err)
return err
}
return backoff.Permanent(err)
}
return nil
}, b); err != nil {
return xerrors.Errorf("retry over: %w", err)
return fmt.Errorf("retry over: %w", err)
}
return nil
}
Expand Down Expand Up @@ -144,7 +145,7 @@ func (u *AuthProxy) run(ctx context.Context, o runOption) error {
eg.Go(func() error {
u.Logger.V(1).Infof("starting a port forwarder")
if err := u.PortForwarder.Run(o.portForwarderOption, portForwarderIsReady, stopPortForwarder); err != nil {
return xerrors.Errorf("could not run a port forwarder: %w", err)
return fmt.Errorf("could not run a port forwarder: %w", err)
}
u.Logger.V(1).Infof("stopped the port forwarder")
if ctx.Err() == nil {
Expand All @@ -158,21 +159,21 @@ func (u *AuthProxy) run(ctx context.Context, o runOption) error {
<-ctx.Done()
u.Logger.V(1).Infof("stopping the port forwarder")
close(stopPortForwarder)
return xerrors.Errorf("context canceled while running the port forwarder: %w", ctx.Err())
return fmt.Errorf("context canceled while running the port forwarder: %w", ctx.Err())
})
// start a reverse proxy when the port forwarder is ready
eg.Go(func() error {
select {
case <-portForwarderIsReady:
u.Logger.V(1).Infof("starting a reverse proxy")
if err := u.ReverseProxy.Run(o.reverseProxyOption, reverseProxyIsReady); err != nil {
return xerrors.Errorf("could not run a reverse proxy: %w", err)
return fmt.Errorf("could not run a reverse proxy: %w", err)
}
u.Logger.V(1).Infof("stopped the reverse proxy")
return nil
case <-ctx.Done():
u.Logger.V(1).Infof("context canceled before starting reverse proxy")
return xerrors.Errorf("context canceled before starting reverse proxy: %w", ctx.Err())
return fmt.Errorf("context canceled before starting reverse proxy: %w", ctx.Err())
}
})
// open the browser when the reverse proxy is ready
Expand All @@ -197,18 +198,18 @@ func (u *AuthProxy) run(ctx context.Context, o runOption) error {
<-ctx.Done()
u.Logger.V(1).Infof("shutting down the reverse proxy")
if err := rp.Shutdown(context.Background()); err != nil {
return xerrors.Errorf("could not shutdown the reverse proxy: %w", err)
return fmt.Errorf("could not shutdown the reverse proxy: %w", err)
}
return xerrors.Errorf("context canceled while running the reverse proxy: %w", ctx.Err())
return fmt.Errorf("context canceled while running the reverse proxy: %w", ctx.Err())
})
return nil
case <-ctx.Done():
u.Logger.V(1).Infof("context canceled before reverse proxy is ready")
return xerrors.Errorf("context canceled before reverse proxy is ready: %w", ctx.Err())
return fmt.Errorf("context canceled before reverse proxy is ready: %w", ctx.Err())
}
})
if err := eg.Wait(); err != nil {
return xerrors.Errorf("error while running an authentication proxy: %w", err)
return fmt.Errorf("error while running an authentication proxy: %w", err)
}
return nil
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/authproxy/auth_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package authproxy

import (
"context"
"errors"
"net/http"
"net/url"
"testing"
Expand All @@ -17,7 +18,6 @@ import (
"github.com/int128/kauthproxy/pkg/reverseproxy"
"github.com/int128/kauthproxy/pkg/reverseproxy/mock_reverseproxy"
"github.com/int128/kauthproxy/pkg/transport"
"golang.org/x/xerrors"
v1 "k8s.io/api/core/v1"
v1meta "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/rest"
Expand Down Expand Up @@ -132,7 +132,7 @@ func TestAuthProxy_Do(t *testing.T) {
BindAddressCandidates: []string{"127.0.0.1:8000"},
}
err := u.Do(ctx, o)
if !xerrors.Is(err, context.DeadlineExceeded) {
if !errors.Is(err, context.DeadlineExceeded) {
t.Errorf("err wants context.DeadlineExceeded but was %+v", err)
}
})
Expand All @@ -142,7 +142,7 @@ func TestAuthProxy_Do(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

portForwarderError := xerrors.New("could not connect to pod")
portForwarderError := errors.New("could not connect to pod")
portForwarder := mock_portforwarder.NewMockInterface(ctrl)
portForwarder.EXPECT().
Run(portforwarder.Option{
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestAuthProxy_Do(t *testing.T) {
BindAddressCandidates: []string{"127.0.0.1:8000"},
}
err := u.Do(ctx, o)
if !xerrors.Is(err, portForwarderError) {
if !errors.Is(err, portForwarderError) {
t.Errorf("err wants the port forwarder error but was %+v", err)
}
})
Expand All @@ -198,7 +198,7 @@ func TestAuthProxy_Do(t *testing.T) {
<-stopChan
return nil
})
reverseProxyError := xerrors.New("could not listen")
reverseProxyError := errors.New("could not listen")
reverseProxy := mock_reverseproxy.NewMockInterface(ctrl)
reverseProxy.EXPECT().
Run(reverseproxy.Option{
Expand Down Expand Up @@ -228,7 +228,7 @@ func TestAuthProxy_Do(t *testing.T) {
BindAddressCandidates: []string{"127.0.0.1:8000"},
}
err := u.Do(ctx, o)
if !xerrors.Is(err, reverseProxyError) {
if !errors.Is(err, reverseProxyError) {
t.Errorf("err wants the port forwarder error but was %+v", err)
}
})
Expand Down Expand Up @@ -303,7 +303,7 @@ func TestAuthProxy_Do(t *testing.T) {
BindAddressCandidates: []string{"127.0.0.1:8000"},
}
err := u.Do(ctx, o)
if !xerrors.Is(err, context.DeadlineExceeded) {
if !errors.Is(err, context.DeadlineExceeded) {
t.Errorf("err wants context.DeadlineExceeded but was %+v", err)
}
})
Expand Down Expand Up @@ -393,7 +393,7 @@ func TestAuthProxy_Do(t *testing.T) {
BindAddressCandidates: []string{"127.0.0.1:8000"},
}
err := u.Do(ctx, o)
if !xerrors.Is(err, context.DeadlineExceeded) {
if !errors.Is(err, context.DeadlineExceeded) {
t.Errorf("err wants context.DeadlineExceeded but was %+v", err)
}
})
Expand Down
4 changes: 2 additions & 2 deletions pkg/browser/browser.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package browser

import (
"fmt"
"github.com/google/wire"
"github.com/pkg/browser"
"golang.org/x/xerrors"
)

var Set = wire.NewSet(
Expand All @@ -22,7 +22,7 @@ type Browser struct{}
// Open opens the default browser.
func (*Browser) Open(url string) error {
if err := browser.OpenURL(url); err != nil {
return xerrors.Errorf("could not open the browser: %w", err)
return fmt.Errorf("could not open the browser: %w", err)
}
return nil
}
13 changes: 7 additions & 6 deletions pkg/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@ package cmd

import (
"context"
"errors"
"fmt"
"net/url"

"github.com/google/wire"
"github.com/int128/kauthproxy/pkg/authproxy"
"github.com/int128/kauthproxy/pkg/logger"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"golang.org/x/xerrors"
"k8s.io/cli-runtime/pkg/genericclioptions"
)

Expand Down Expand Up @@ -43,7 +44,7 @@ func (cmd *Cmd) Run(ctx context.Context, osArgs []string, version string) int {

rootCmd.SetArgs(osArgs[1:])
if err := rootCmd.ExecuteContext(ctx); err != nil {
if xerrors.Is(err, context.Canceled) {
if errors.Is(err, context.Canceled) {
cmd.Logger.V(1).Infof("terminating: %s", err)
return 0
}
Expand Down Expand Up @@ -95,15 +96,15 @@ All traffic is routed by the authentication proxy and port forwarder as follows:
func (cmd *Cmd) runRootCmd(ctx context.Context, o rootCmdOptions, args []string) error {
remoteURL, err := url.Parse(args[0])
if err != nil {
return xerrors.Errorf("invalid remote URL: %w", err)
return fmt.Errorf("invalid remote URL: %w", err)
}
config, err := o.k8sOptions.ToRESTConfig()
if err != nil {
return xerrors.Errorf("could not load the config: %w", err)
return fmt.Errorf("could not load the config: %w", err)
}
namespace, _, err := o.k8sOptions.ToRawKubeConfigLoader().Namespace()
if err != nil {
return xerrors.Errorf("could not determine the namespace: %w", err)
return fmt.Errorf("could not determine the namespace: %w", err)
}
authProxyOption := authproxy.Option{
Config: config,
Expand All @@ -113,7 +114,7 @@ func (cmd *Cmd) runRootCmd(ctx context.Context, o rootCmdOptions, args []string)
SkipOpenBrowser: o.skipOpenBrowser,
}
if err := cmd.AuthProxy.Do(ctx, authProxyOption); err != nil {
return xerrors.Errorf("could not run an authentication proxy: %w", err)
return fmt.Errorf("could not run an authentication proxy: %w", err)
}
return nil
}
6 changes: 3 additions & 3 deletions pkg/env/env.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package env

import (
"fmt"
"net"

"github.com/google/wire"
"golang.org/x/xerrors"
)

var Set = wire.NewSet(
Expand All @@ -24,12 +24,12 @@ type Env struct{}
func (*Env) AllocateLocalPort() (int, error) {
l, err := net.Listen("tcp", "localhost:0")
if err != nil {
return 0, xerrors.Errorf("could not listen: %w", err)
return 0, fmt.Errorf("could not listen: %w", err)
}
defer l.Close()
addr, ok := l.Addr().(*net.TCPAddr)
if !ok {
return 0, xerrors.Errorf("internal error: unknown type %T", l.Addr())
return 0, fmt.Errorf("internal error: unknown type %T", l.Addr())
}
return addr.Port, nil
}
9 changes: 4 additions & 5 deletions pkg/portforwarder/port_forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"os"

"github.com/google/wire"
"golang.org/x/xerrors"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/portforward"
"k8s.io/client-go/transport/spdy"
Expand Down Expand Up @@ -47,20 +46,20 @@ type PortForwarder struct {
func (pf *PortForwarder) Run(o Option, readyChan chan struct{}, stopChan <-chan struct{}) error {
pfURL, err := url.Parse(fmt.Sprintf("%s/api/v1/namespaces/%s/pods/%s/portforward", o.Config.Host, o.TargetNamespace, o.TargetPodName))
if err != nil {
return xerrors.Errorf("could not build URL for portforward: %w", err)
return fmt.Errorf("could not build URL for portforward: %w", err)
}
rt, upgrader, err := spdy.RoundTripperFor(o.Config)
if err != nil {
return xerrors.Errorf("could not create a round tripper: %w", err)
return fmt.Errorf("could not create a round tripper: %w", err)
}
dialer := spdy.NewDialer(upgrader, &http.Client{Transport: rt}, http.MethodPost, pfURL)
portPair := fmt.Sprintf("%d:%d", o.SourcePort, o.TargetContainerPort)
forwarder, err := portforward.NewOnAddresses(dialer, []string{"127.0.0.1"}, []string{portPair}, stopChan, readyChan, os.Stdout, os.Stderr)
if err != nil {
return xerrors.Errorf("could not create a port forwarder: %w", err)
return fmt.Errorf("could not create a port forwarder: %w", err)
}
if err := forwarder.ForwardPorts(); err != nil {
return xerrors.Errorf("could not run the port forwarder at %s: %w", portPair, err)
return fmt.Errorf("could not run the port forwarder at %s: %w", portPair, err)
}
return nil
}
Loading

0 comments on commit 4be070c

Please sign in to comment.