-
Notifications
You must be signed in to change notification settings - Fork 560
Introduce GRPC_PROXY EnvVar Support #2364
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,17 @@ package grpc | |
|
||
import ( | ||
"context" | ||
"net" | ||
"net/url" | ||
"os" | ||
"sync" | ||
"time" | ||
|
||
"github.com/operator-framework/operator-registry/pkg/client" | ||
|
||
"github.com/sirupsen/logrus" | ||
"golang.org/x/net/http/httpproxy" | ||
"golang.org/x/net/proxy" | ||
"google.golang.org/grpc" | ||
"google.golang.org/grpc/connectivity" | ||
|
||
|
@@ -100,10 +105,71 @@ func (s *SourceStore) Get(key registry.CatalogKey) *SourceConn { | |
return &source | ||
} | ||
|
||
func grpcProxyURL(addr string) (*url.URL, error) { | ||
// Handle ip addresses | ||
host, _, err := net.SplitHostPort(addr) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
url, err := url.Parse(host) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// Hardcode fields required for proxy resolution | ||
url.Host = addr | ||
url.Scheme = "http" | ||
|
||
// Override HTTPS_PROXY and HTTP_PROXY with GRPC_PROXY | ||
proxyConfig := &httpproxy.Config{ | ||
HTTPProxy: getGRPCProxyEnv(), | ||
HTTPSProxy: getGRPCProxyEnv(), | ||
NoProxy: getEnvAny("NO_PROXY", "no_proxy"), | ||
CGI: os.Getenv("REQUEST_METHOD") != "", | ||
Comment on lines
+126
to
+129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: We should probably memoize these so we don't fetch them for every dial. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems reasonable but would make the values static, which likely isn't a problem given how deployments and pods work. |
||
} | ||
njhale marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Check if a proxy should be used based on environment variables | ||
return proxyConfig.ProxyFunc()(url) | ||
} | ||
|
||
func getGRPCProxyEnv() string { | ||
return getEnvAny("GRPC_PROXY", "grpc_proxy") | ||
} | ||
|
||
func getEnvAny(names ...string) string { | ||
tylerslaton marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for _, n := range names { | ||
if val := os.Getenv(n); val != "" { | ||
return val | ||
} | ||
} | ||
return "" | ||
} | ||
|
||
func grpcConnection(address string) (*grpc.ClientConn, error) { | ||
dialOptions := []grpc.DialOption{grpc.WithInsecure()} | ||
proxyURL, err := grpcProxyURL(address) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
if proxyURL != nil { | ||
dialOptions = append(dialOptions, grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { | ||
dialer, err := proxy.FromURL(proxyURL, &net.Dialer{}) | ||
if err != nil { | ||
return nil, err | ||
} | ||
return dialer.Dial("tcp", addr) | ||
})) | ||
} | ||
|
||
return grpc.Dial(address, dialOptions...) | ||
} | ||
|
||
func (s *SourceStore) Add(key registry.CatalogKey, address string) (*SourceConn, error) { | ||
_ = s.Remove(key) | ||
|
||
conn, err := grpc.Dial(address, grpc.WithInsecure()) | ||
conn, err := grpcConnection(address) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we abstract the dial logic away? maybe take a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we pursue this option wouldn't the connection to the proxy need to remain open indefinitely? |
||
if err != nil { | ||
return nil, err | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.