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.

Sign up for GitHub

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

Expand proxy detection support to Dex and support proxy configuration #3636

Merged
merged 2 commits into from
Dec 11, 2024
Merged

Expand proxy detection support to Dex and support proxy configuration #3636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea51715
Update the dex controller for proxy detection
pasanw Dec 5, 2024
638961d
Support user-provided proxy env vars
pasanw Dec 5, 2024
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
71 changes: 71 additions & 0 deletions api/v1/installation_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,12 @@ type InstallationSpec struct {
// Azure is used to configure azure provider specific options.
// +optional
Azure *Azure `json:"azure,omitempty"`

// Proxy is used to configure the HTTP(S) proxy settings that will be applied to Tigera containers that connect
// to destinations outside the cluster. It is expected that NO_PROXY is configured such that destinations within
// the cluster (including the API server) are exempt from proxying.
// +optional
Proxy *Proxy `json:"proxy,omitempty"`
rene-dekker marked this conversation as resolved.
Show resolved Hide resolved
}

type Azure struct {
Expand Down Expand Up @@ -1048,3 +1054,68 @@ type WindowsNodeSpec struct {
// +optional
VXLANAdapter string `json:"vxlanAdapter,omitempty"`
}

type Proxy struct {
// HTTPProxy defines the value of the HTTP_PROXY environment variable that will be set on Tigera containers that connect to
// destinations outside the cluster.
// +optional
HTTPProxy string `json:"httpProxy,omitempty"`

// HTTPSProxy defines the value of the HTTPS_PROXY environment variable that will be set on Tigera containers that connect to
// destinations outside the cluster.
// +optional
HTTPSProxy string `json:"httpsProxy,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Not to throw a wrench into things, but since these vars can take a separated list of items should we make them lists in our API?

httpsProxy:
- foo.com
- bar.org

and combine them? Or nah?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

httpProxy and httpsProxy should always be a single value, not a list AFAIK. noProxy can be a list of exempted destinations, but it's expected that this is a comma-separated list. If we ask for an array, it will likely be an inconvenience for the user IMO - they've likely been passing a comma-separated list elsewhere

Copy link
Member

Choose a reason for hiding this comment

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

ah right, it was noProxy I was thinking about. I'm fine with the stance that it should just be the same as the env var.


// NoProxy defines the value of the NO_PROXY environment variable that will be set on Tigera containers that connect to
// destinations outside the cluster. This value must be set such that destinations within the scope of the cluster, including
// the Kubernetes API server, are exempt from being proxied.
// +optional
NoProxy string `json:"noProxy,omitempty"`
}

func (p *Proxy) EnvVars() (envVars []v1.EnvVar) {
if p == nil {
return
}

if p.HTTPProxy != "" {
envVars = append(envVars, []v1.EnvVar{
{
Name: "HTTP_PROXY",
Value: p.HTTPProxy,
},
{
Name: "http_proxy",
Value: p.HTTPProxy,
},
}...)
}

if p.HTTPSProxy != "" {
envVars = append(envVars, []v1.EnvVar{
{
Name: "HTTPS_PROXY",
Value: p.HTTPSProxy,
},
{
Name: "https_proxy",
Value: p.HTTPSProxy,
},
}...)
}

if p.NoProxy != "" {
envVars = append(envVars, []v1.EnvVar{
{
Name: "NO_PROXY",
Value: p.NoProxy,
},
{
Name: "no_proxy",
Value: p.NoProxy,
},
}...)
}

return envVars
}
20 changes: 20 additions & 0 deletions api/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

92 changes: 85 additions & 7 deletions pkg/controller/authentication/authentication_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import (
"context"
"fmt"

"golang.org/x/net/http/httpproxy"
Copy link
Member

Choose a reason for hiding this comment

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

nit: the imports in this file are a bit of a mess

v1 "k8s.io/api/apps/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/client-go/kubernetes"

"github.com/tigera/operator/pkg/render/common/networkpolicy"
Expand Down Expand Up @@ -156,13 +159,15 @@ var _ reconcile.Reconciler = &ReconcileAuthentication{}

// ReconcileAuthentication reconciles an Authentication object
type ReconcileAuthentication struct {
client client.Client
scheme *runtime.Scheme
provider oprv1.Provider
status status.StatusManager
clusterDomain string
tierWatchReady *utils.ReadyFlag
multiTenant bool
client client.Client
scheme *runtime.Scheme
provider oprv1.Provider
status status.StatusManager
clusterDomain string
tierWatchReady *utils.ReadyFlag
multiTenant bool
resolvedPodProxies []*httpproxy.Config
lastAvailabilityTransition metav1.Time
}

// Reconcile the cluster state with the Authentication object that is found in the cluster.
Expand Down Expand Up @@ -306,6 +311,78 @@ func (r *ReconcileAuthentication) Reconcile(ctx context.Context, request reconci
return reconcile.Result{}, err
}

// Determine the current deployment availability.
var currentAvailabilityTransition metav1.Time
var currentlyAvailable bool
dexDeployment := v1.Deployment{}
err = r.client.Get(ctx, client.ObjectKey{Name: render.DexObjectName, Namespace: render.DexNamespace}, &dexDeployment)
if err != nil && !errors.IsNotFound(err) {
r.status.SetDegraded(oprv1.ResourceReadError, "Failed to read the deployment status of Dex", err, reqLogger)
return reconcile.Result{}, nil
} else if err == nil {
for _, condition := range dexDeployment.Status.Conditions {
if condition.Type == v1.DeploymentAvailable {
currentAvailabilityTransition = condition.LastTransitionTime
if condition.Status == corev1.ConditionTrue {
currentlyAvailable = true
}
break
}
}
}

// Resolve the proxies used by each Dex pod. We only update the resolved proxies if the availability of the
// Dex deployment has changed since our last reconcile and the deployment is currently available. We restrict
// the resolution of pod proxies in this way to limit the number of pod queries we make.
if !currentAvailabilityTransition.Equal(&r.lastAvailabilityTransition) && currentlyAvailable {
// Query dex pods.
labelSelector := labels.SelectorFromSet(map[string]string{
"app.kubernetes.io/name": render.DexObjectName,
})
pods := corev1.PodList{}
err := r.client.List(ctx, &pods, &client.ListOptions{
LabelSelector: labelSelector,
Namespace: render.DexNamespace,
})
if err != nil {
r.status.SetDegraded(oprv1.ResourceReadError, "Failed to list the pods of the Dex deployment", err, reqLogger)
return reconcile.Result{}, nil
}

// Resolve the proxy config for each pod. Pods without a proxy will have a nil proxy config value.
var podProxies []*httpproxy.Config
for _, pod := range pods.Items {
for _, container := range pod.Spec.Containers {
if container.Name == render.DexObjectName {
var podProxyConfig *httpproxy.Config
var httpProxy, httpsProxy, noProxy string
for _, env := range container.Env {
switch env.Name {
case "http_proxy", "HTTP_PROXY":
httpProxy = env.Value
case "https_proxy", "HTTPS_PROXY":
httpsProxy = env.Value
case "no_proxy", "NO_PROXY":
noProxy = env.Value
}
}
if httpProxy != "" || httpsProxy != "" || noProxy != "" {
podProxyConfig = &httpproxy.Config{
HTTPProxy: httpProxy,
HTTPSProxy: httpsProxy,
NoProxy: noProxy,
}
}

podProxies = append(podProxies, podProxyConfig)
}
}
}

r.resolvedPodProxies = podProxies
}
r.lastAvailabilityTransition = currentAvailabilityTransition

Comment on lines +314 to +385
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For context, this logic is a mirror of what is found in the cluster connection controller

disableDex := utils.IsDexDisabled(authentication)

// DexConfig adds convenience methods around dex related objects in k8s and can be used to configure Dex.
Expand All @@ -324,6 +401,7 @@ func (r *ReconcileAuthentication) Reconcile(ctx context.Context, request reconci
TLSKeyPair: tlsKeyPair,
TrustedBundle: trustedBundle,
Authentication: authentication,
PodProxies: r.resolvedPodProxies,
}

// Render the desired objects from the CRD and create or update them.
Expand Down
Loading
Loading