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

Standard Service configuration #7

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions environment/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ const (
commaSeparator = ","
)

var (
ErrNotFound = errors.New("Environment variable not found")
)

// GetLogLevel returns the loglevet or panics. This is called before any logger
// is available. i.e. don't use a logger here.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have previously established the OrFatal convention because some people object to functions which terminate. I understand we cant use logger.Fatalf here because this must be called before it is created. I think OrPanic would be better aligned with established patterns.

Personally, I have no problem with declaring that the environment module is 'allowed to panic or fatal out' because it is designed to be used as part of pre-flight startup. And then remove all the OrWhatever suffixes.

But we should get everyone on board with that before doing so.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is a speculative change only and may go away...

func GetLogLevel() string {
Expand Down
128 changes: 128 additions & 0 deletions grpcclient/client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package grpcclient

import (
"errors"
"fmt"

"google.golang.org/grpc"
"google.golang.org/grpc/credentials/insecure"
)

var (
ErrConflictingOptions = errors.New("Conflicting options")
)

type ClientConn = grpc.ClientConn

type GRPCClient struct {
name string
log Logger
address string
unaryInterceptor bool
unaryInterceptorNoAuth bool

conn *ClientConn
}

// Factory API
type GRPCClientOption func(*GRPCClient)

// UnaryInterceptor is always used unless disabled by this call.
func WithoutUnaryInterceptor() GRPCClientOption {
return func(t *GRPCClient) {
t.unaryInterceptor = false
}
}

// UnaryInterceptorNoAuth is used instead of UnaryInterceptor.
func WithNoAuthUnaryInterceptor() GRPCClientOption {
return func(t *GRPCClient) {
t.unaryInterceptorNoAuth = true
}
}

func NewGRPCClient(log Logger, name string, address string, opts ...GRPCClientOption) GRPCClient {
t := GRPCClient{
log: log,
name: name,
address: address,
unaryInterceptor: true, // default is always present
unaryInterceptorNoAuth: false, // default is not present
}
for _, opt := range opts {
opt(&t)
}
return t
}

// Open - makes the connection using the provided attributes. This is idempotent.
func (g *GRPCClient) Open() error {
var err error
var conn *grpc.ClientConn

// Idempotent
if g.conn != nil {
return nil
}

if g.address == "" {
return fmt.Errorf("%s: address is blank", g.name)
}
if g.unaryInterceptorNoAuth && g.unaryInterceptor {
return fmt.Errorf("%s: unaryInterceptor and unaryInterceprorNoAuth are both set: %w", g.name, ErrConflictingOptions)
}

// The default interceptors are:
//
// grpc.WithTransportCredentials(insecure.NewCredentials()),
// grpc.WithUnaryInterceptor(grpcclient.InternalServiceClientUnaryInterceptor()),
//
// alternatively we can have:
//
// grpc.WithTransportCredentials(insecure.NewCredentials()),
// grpc.WithUnaryInterceptor(grpcclient.InternalServiceClientUnaryInterceptorNoAuth()),
//
// OR
//
// grpc.WithTransportCredentials(insecure.NewCredentials()),
//
g.log.Debugf("Open %s client at %v", g.name, g.address)
// If the interceptor should be used in every service without exception then add it to
// the opts list here.
opts := []grpc.DialOption{
grpc.WithTransportCredentials(insecure.NewCredentials()),
}
if g.unaryInterceptorNoAuth {
g.log.Debugf("Open %s client with UnaryInterceptorNoAuth", g.name)
opts = append(
opts,
grpc.WithUnaryInterceptor(InternalServiceClientUnaryInterceptorNoAuth()),
)
}
if g.unaryInterceptor {
g.log.Debugf("Open %s client with UnaryInterceptor", g.name)
opts = append(
opts,
grpc.WithUnaryInterceptor(InternalServiceClientUnaryInterceptor()),
)
}
conn, err = grpc.Dial(g.address, opts...)
if err != nil {
return err
}
g.conn = conn
g.log.Debugf("Open %s client successful", g.name)
return nil
}

// Close - should be deferred. This function is idempotent.
func (g *GRPCClient) Close() {
g.conn = nil
}

func (g *GRPCClient) String() string {
return g.name
}
func (g *GRPCClient) Connection() *ClientConn {
return g.conn
}
8 changes: 8 additions & 0 deletions grpcclient/interface.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package grpcclient

type GRPCClientProvider interface {
Open() error
Close()
String() string
Connection() *ClientConn
}
7 changes: 7 additions & 0 deletions grpcclient/logger.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package grpcclient

import (
"github.com/rkvst/go-rkvstcommon/logger"
)

type Logger = logger.Logger
123 changes: 123 additions & 0 deletions grpcclient/middleware.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package grpcclient

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the metadata constants and some of the functions are not naturaly part of the client module

I have a middleware package in my branch of go-rkvstcommon for things like this

https://github.com/rkvst/go-rkvstcommon/tree/dev/robin/8337-common-metadata/middleware

I think grpcclient is fine in general for things that are definitely only the concern of client users and implementors

import (
"context"
"fmt"

grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
grpc_otrace "github.com/grpc-ecosystem/go-grpc-middleware/tracing/opentracing"
"google.golang.org/grpc"
"google.golang.org/grpc/metadata"

"github.com/rkvst/go-rkvstcommon/logger"
"github.com/rkvst/go-rkvstcommon/tracing"
)

// RFC 6648 should apply. Section 3.3 forbids the use of field names starting with 'x-'
// TODO: remove x- and use archivist throughout...
const (
// ArchivistInternalMetadataKey is set for internal calls, but not for edge calls
ArchivistInternalMetadataKey = "x-archivist-internal"
// ArchivistInternalMetadataValue is the value to set / check
ArchivistInternalMetadataValue = "true"

AuthorizationHeaderKey = "authorization"
)

// propagateMetadataClientUnaryInterceptor passes all metadata from the
// incoming metadata to the outgoing metadata for each downstream request.
func propagateMetadataClientUnaryInterceptor(
ctx context.Context,
method string,
req, reply any,
cc *grpc.ClientConn,
invoker grpc.UnaryInvoker,
opts ...grpc.CallOption,
) error {
md, ok := metadata.FromIncomingContext(ctx)
if !ok {
logger.Sugar.Errorf("Unable to get metadata from context")
return fmt.Errorf("could not get incoming metadata (for metadata forwarding)")
}
// Should not be modified, may cause races
// https://github.com/grpc/grpc-go/blob/89faf1c3e8283dd3c863b877bcf1631d1fe6f50c/metadata/metadata.go#L166
md = md.Copy()

// For the moment we pass everything though, and add the 'internal' key
// We may decide to strip any content that does not have the prefix
// ArchivistInternalMetadataPrefix at some point
md.Set(ArchivistInternalMetadataKey, ArchivistInternalMetadataValue)

traceID := md.Get(tracing.TraceID)
if len(traceID) > 0 {
logger.Sugar.Debugf("%s '%v'", tracing.TraceID, traceID)
}
// We use NewOutgoingContext() to make sure we have a clean, empty outgoing context
// for each call as we don't want any uncontrolled content coming in from the context.
// Other chained interceptors can be added, if so they must use
// metadata.AppendToOutgoingContext() to add extra content.
newCtx := metadata.NewOutgoingContext(ctx, md)

return invoker(newCtx, method, req, reply, cc, opts...)
}

// stripAuthClientUnaryInterceptor strips authentication metadata from
// outgoing metadata for each downstream request.
// XXX: It should go away when we implement User Story 3010
func stripAuthClientUnaryInterceptor(
ctx context.Context,
method string,
req, reply any,
cc *grpc.ClientConn,
invoker grpc.UnaryInvoker,
opts ...grpc.CallOption,
) error {
md, ok := metadata.FromOutgoingContext(ctx)
if !ok {
logger.Sugar.Errorf("Unable to get metadata from context")
return fmt.Errorf("could not get outgoing metadata (for auth striping)")
}
// Should not be modified, may cause races
// https://github.com/grpc/grpc-go/blob/89faf1c3e8283dd3c863b877bcf1631d1fe6f50c/metadata/metadata.go#L166
md = md.Copy()

// remove the authorization (authentication) headers
// we leave the rest (including the tenant ID, intact)
delete(md, AuthorizationHeaderKey)

// We use NewOutgoingContext() to make sure we have a clean, empty outgoing context
// for each call as we don't want any uncontrolled content coming in from the context.
// Other chained interceptors can be added, if so they must use
// metadata.AppendToOutgoingContext() to add extra content.
newCtx := metadata.NewOutgoingContext(ctx, md)

return invoker(newCtx, method, req, reply, cc, opts...)
}

// InternalServiceClientUnaryInterceptor returns a client interceptor that
// pass all metadata from the incoming metadata to the outgoing metadata for
// each downstream request and adds open tracing span and headers.
// This should be used as a grpc Dial option for _all_ internal services and
// not for _any_ external services
func InternalServiceClientUnaryInterceptor() grpc.UnaryClientInterceptor {
return grpc_middleware.ChainUnaryClient(
propagateMetadataClientUnaryInterceptor,
grpc_otrace.UnaryClientInterceptor(),
)
}

// InternalServiceClientUnaryInterceptorNoAuth returns a client interceptor that
// pass some metadata from the incoming metadata to the outgoing metadata for
// each downstream request and adds open tracing span and headers.
// It does NOT include the authorisation metadata, but it DOES include all the other
// standard contents.
//
// This should ONLY be used as a grpc Dial option for internal sercices that need
// to make calls that skip PDP (ABAC) authorisation.
// XXX: It should go away when we implement User Story 3010
func InternalServiceClientUnaryInterceptorNoAuth() grpc.UnaryClientInterceptor {
return grpc_middleware.ChainUnaryClient(
InternalServiceClientUnaryInterceptor(),
stripAuthClientUnaryInterceptor,
)
}
7 changes: 7 additions & 0 deletions startup/grpcclient.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package startup

import (
"github.com/rkvst/go-rkvstcommon/grpcclient"
)

robinbryce marked this conversation as resolved.
Show resolved Hide resolved
type GRPCClient = grpcclient.GRPCClientProvider
Loading