Skip to content
Open
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
132 changes: 74 additions & 58 deletions pkg/framework/proxies.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,14 @@ import (
)

const (
mapiControllersDeploymentName = "machine-api-controllers"
machineControllerContainerName string = "machine-controller"
proxyNamespace = MachineAPINamespace
proxyName = "mitm-proxy"
mitmSignerName = "mitm-signer"
mitmBootstrapName = "mitm-bootstrap"
mitmCustomPKIName = "mitm-custom-pki"
mitmCustomPKINamespace = "openshift-config"
mitmDaemonsetName = proxyName
mitmServiceName = proxyName
proxyNamespace = MachineAPINamespace
proxyName = "mitm-proxy"
mitmSignerName = "mitm-signer"
mitmBootstrapName = "mitm-bootstrap"
mitmCustomPKIName = "mitm-custom-pki"
mitmCustomPKINamespace = "openshift-config"
mitmDaemonsetName = proxyName
mitmServiceName = proxyName
)

const proxySetup = `
Expand All @@ -65,6 +63,16 @@ tar xvf mitmproxy-5.3.0-linux.tar.gz
HOME=/.mitmproxy ./mitmdump --ssl-insecure
`

// IsClusterProxyEnabled returns true if the cluster has a global proxy enabled.
func IsClusterProxyEnabled(ctx context.Context, c client.Client) (bool, error) {
proxy := &configv1.Proxy{}
if err := c.Get(ctx, client.ObjectKey{Name: "cluster"}, proxy); err != nil {
return false, fmt.Errorf("failed to get cluster proxy: %w", err)
}

return len(proxy.Status.HTTPProxy) > 0 || len(proxy.Status.HTTPSProxy) > 0, nil
}

// DeployProxy deploys a MITM Proxy to the cluster.
func DeployProxy(c client.Client, gomegaArgs ...interface{}) {
ctx := context.Background()
Expand Down Expand Up @@ -142,42 +150,11 @@ func ConfigureClusterWideProxy(c client.Client, gomegaArgs ...interface{}) {
Name: mitmCustomPKIName,
}
}), gomegaArgs...).Should(Succeed(), "cluster wide proxy set be able to be updated")

deploy := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: mapiControllersDeploymentName,
Namespace: proxyNamespace,
},
}

By("Waiting for machine-api-controller deployment to reflect configured cluster-wide proxy")

Eventually(kom.Object(deploy), time.Minute*5).Should(
HaveField("Spec.Template.Spec.Containers", ContainElement(SatisfyAll(
HaveField("Name", Equal(machineControllerContainerName)),
HaveField("Env", SatisfyAll(
ContainElement(SatisfyAll(
HaveField("Name", "NO_PROXY"),
HaveField("Value", Not(BeEmpty())),
)),
ContainElement(SatisfyAll(
HaveField("Name", "HTTPS_PROXY"),
HaveField("Value", Not(BeEmpty())),
)),
ContainElement(SatisfyAll(
HaveField("Name", "HTTP_PROXY"),
HaveField("Value", Not(BeEmpty())),
)),
)),
))),
"Cluster-wide proxy environment variables were not set.",
)
}

// UnconfigureClusterWideProxy configures the Cluster-Wide Proxy to stop using the MITM Proxy.
func UnconfigureClusterWideProxy(c client.Client, gomegaArgs ...interface{}) {
ctx := context.Background()
kom := komega.New(c)

proxy := &configv1.Proxy{}
Eventually(c.Get(ctx, client.ObjectKey{Name: "cluster"}, proxy)).Should(Succeed(), "timed out getting Proxy named 'cluster.'")
Expand All @@ -188,31 +165,70 @@ func UnconfigureClusterWideProxy(c client.Client, gomegaArgs ...interface{}) {
{"op": "remove", "path": "/spec/noProxy"},
{"op": "remove", "path": "/spec/trustedCA"}
]`)))).Should(Succeed(), "timed out patching Proxy Spec.")
}

// WaitForAllContainersProxyEnvVars waits for all containers in the given Deployment
// to have HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables set.
func WaitForAllContainersProxyEnvVars(c client.Client, namespace, deploymentName string) {
kom := komega.New(c)

deploy := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: mapiControllersDeploymentName,
Namespace: proxyNamespace,
Name: deploymentName,
Namespace: namespace,
},
}

By("Waiting for machine-api-controller deployment to reflect unconfigured cluster-wide proxy")
proxyEnvMatcher := SatisfyAll(
ContainElement(SatisfyAll(
HaveField("Name", "NO_PROXY"),
HaveField("Value", Not(BeEmpty())),
)),
ContainElement(SatisfyAll(
HaveField("Name", "HTTPS_PROXY"),
HaveField("Value", Not(BeEmpty())),
)),
ContainElement(SatisfyAll(
HaveField("Name", "HTTP_PROXY"),
HaveField("Value", Not(BeEmpty())),
)),
)

By(fmt.Sprintf("Waiting for all containers in %s/%s to reflect configured cluster-wide proxy", namespace, deploymentName))

Eventually(kom.Object(deploy), time.Minute*5).Should(
HaveField("Spec.Template.Spec.Containers", Not(ContainElement(
HaveField("Env", Not(proxyEnvMatcher)),
))),
fmt.Sprintf("Cluster-wide proxy environment variables were not set on all containers of %s/%s.", namespace, deploymentName),
)
}

// WaitForAllContainersNoProxyEnvVars waits for all containers in the given Deployment
// to have HTTP_PROXY, HTTPS_PROXY, and NO_PROXY environment variables removed.
func WaitForAllContainersNoProxyEnvVars(c client.Client, namespace, deploymentName string) {
kom := komega.New(c)

deploy := &appsv1.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: deploymentName,
Namespace: namespace,
},
}

noProxyEnvMatcher := SatisfyAll(
Not(ContainElement(HaveField("Name", "NO_PROXY"))),
Not(ContainElement(HaveField("Name", "HTTPS_PROXY"))),
Not(ContainElement(HaveField("Name", "HTTP_PROXY"))),
)

By(fmt.Sprintf("Waiting for all containers in %s/%s to reflect unconfigured cluster-wide proxy", namespace, deploymentName))

Eventually(kom.Object(deploy), time.Minute*5).Should(
HaveField("Spec.Template.Spec.Containers", ContainElement(SatisfyAll(
HaveField("Name", Equal(machineControllerContainerName)),
HaveField("Env", SatisfyAll(
Not(ContainElement(SatisfyAll(
HaveField("Name", "NO_PROXY"),
))),
Not(ContainElement(SatisfyAll(
HaveField("Name", "HTTPS_PROXY"),
))),
Not(ContainElement(SatisfyAll(
HaveField("Name", "HTTP_PROXY"),
))),
)),
HaveField("Spec.Template.Spec.Containers", Not(ContainElement(
HaveField("Env", Not(noProxyEnvMatcher)),
))),
"Cluster-wide proxy environmenet variables were still set.",
fmt.Sprintf("Cluster-wide proxy environment variables were still set on some containers of %s/%s.", namespace, deploymentName),
)
}

Expand Down
133 changes: 133 additions & 0 deletions pkg/operators/cluster-capi-operator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package operators

import (
"context"
"fmt"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

configv1 "github.com/openshift/api/config/v1"
appsv1 "k8s.io/api/apps/v1"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"

"github.com/openshift/cluster-api-actuator-pkg/pkg/framework"
"github.com/openshift/cluster-api-actuator-pkg/pkg/framework/gatherer"
)

var _ = Describe(
"[sig-cluster-lifecycle] Cluster API When cluster-wide proxy is configured, Cluster API provider deployments should",
framework.LabelDisruptive, framework.LabelConnectedOnly, framework.LabelPeriodic, framework.LabelCAPI,
Comment on lines +19 to +20

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know how to review this label, but I suspect it's important. @damdo ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I referenced MAPI's:

"[sig-cluster-lifecycle] Machine API When cluster-wide proxy is configured, Machine API cluster operator should ",
framework.LabelDisruptive, framework.LabelConnectedOnly, framework.LabelPeriodic, framework.LabelMAPI,

Custom labels being defined: https://github.com/openshift/cluster-api-actuator-pkg/blob/master/pkg/framework/ginkgo-labels.go

Serial,
func() {
var (
gatherer *gatherer.StateGatherer
client runtimeclient.Client
ctx context.Context
isProxyJob bool
)

BeforeEach(func() {
var err error

client, err = framework.LoadClient()
Expect(err).NotTo(HaveOccurred(), "Failed to load client")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Skip if the CAPIMachineManagement (or whatever) feature gate is not set?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Assuming the goal here is to skip test execution on non-applicable clusters (CAPI is not enabled):

I checked how existing tests handle similar situations. They simply call framework.SkipIfNotTechPreviewNoUpgradeCtx() in BeforeAll: https://github.com/search?q=repo%3Aopenshift%2Fcluster-api-actuator-pkg++SkipIfNotTechPreviewNoUpgradeCtx+path%3A%2F%5Epkg%5C%2Fcapi%5C%2F%2F&type=code

I can add the same here. As for a more granular check (by feature set name, like CAPIMachineManagement), we can revisit/refactor that in the future once CAPI is GA'd.


ctx = framework.GetContext()

gatherer, err = framework.NewGatherer()
Expect(err).ToNot(HaveOccurred(), "Failed to load gatherer")

isProxyJob, err = framework.IsClusterProxyEnabled(ctx, client)
Expect(err).ToNot(HaveOccurred(), "Failed to check cluster proxy configuration")

if isProxyJob {
By("cluster-wide proxy is already configured on proxy cluster, skipping MITM proxy deployment")
} else {
By("deploying an HTTP proxy")
framework.DeployProxy(client)

By("configuring cluster-wide proxy")
framework.ConfigureClusterWideProxy(client)
}
})

It("have proxy environment variables injected", func() {
By("waiting for Cluster API provider deployments to reflect proxy configuration")

deployments := &appsv1.DeploymentList{}
Eventually(client.List(ctx, deployments,
runtimeclient.InNamespace(framework.ClusterAPINamespace),
runtimeclient.HasLabels{"cluster.x-k8s.io/provider"},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we should continue to rely on these labels. Maybe the installer revision label? That would unambiguously identify any CAPI operand.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Just to confirm my understanding: are you suggesting using newly added capi-operator.openshift.io/managed-by ones?

openshift/cluster-capi-operator@4c058ec#diff-5baf795a0dd949a76f806079a3a3ed7094e626eb090207d761d2a91c5c1a9311R30-R31

)).Should(Succeed(), "timed out listing Cluster API provider Deployments.")
Expect(deployments.Items).NotTo(BeEmpty(), "no Cluster API provider Deployments found")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For extra robustness I'd add a wait, probably in BeforeEach, that generation == observedRevisionGeneration and desiredRevision == currentRevision on the ClusterAPI object.


for _, deploy := range deployments.Items {
By(fmt.Sprintf("verifying proxy env vars on Deployment %s", deploy.Name))
framework.WaitForAllContainersProxyEnvVars(client, framework.ClusterAPINamespace, deploy.Name)
}
})

AfterEach(func() {
specReport := CurrentSpecReport()
if specReport.Failed() {
Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "Failed to GatherAll")
}

if isProxyJob {
By("cluster-wide proxy was pre-existing, skipping MITM proxy cleanup")
return
}

By("unconfiguring cluster-wide proxy")
framework.UnconfigureClusterWideProxy(client)

var err error

client, err = framework.LoadClient()
Expect(err).NotTo(HaveOccurred(), "Failed to refresh client after proxy teardown")

By("verifying proxy env vars are removed from Cluster API provider Deployments")

deployments := &appsv1.DeploymentList{}
Eventually(client.List(ctx, deployments,
runtimeclient.InNamespace(framework.ClusterAPINamespace),
runtimeclient.HasLabels{"cluster.x-k8s.io/provider"},
)).Should(Succeed(), "timed out listing Cluster API provider Deployments.")
Expect(deployments.Items).NotTo(BeEmpty(), "no Cluster API provider Deployments found")

for _, deploy := range deployments.Items {
By(fmt.Sprintf("verifying proxy env vars removed from Deployment %s", deploy.Name))
framework.WaitForAllContainersNoProxyEnvVars(client, framework.ClusterAPINamespace, deploy.Name)
}

By("waiting for KAPI cluster operator to become available")
Expect(framework.WaitForStatusAvailableOverLong(ctx, client, "kube-apiserver")).To(BeTrue(),
"Failed to wait for kube-apiserver Cluster Operator to become available")

By("waiting for KCM cluster operator to become available")
Expect(framework.WaitForStatusAvailableOverLong(ctx, client, "kube-controller-manager")).To(BeTrue(),
"Failed to wait for kube-controller-manager Cluster Operator to become available")

By("waiting for cluster-api cluster operator to become available")
Expect(framework.WaitForStatusAvailableMedium(ctx, client, "cluster-api")).To(BeTrue(),
"Failed to wait for cluster-api Cluster Operator to become available")

By("waiting for all nodes to become ready")
Expect(framework.WaitUntilAllNodesAreReady(ctx, client)).To(Succeed(),
"Failed to wait for all nodes to become ready")

By("waiting for all cluster operators to become available")

coList := &configv1.ClusterOperatorList{}
Eventually(client.List(ctx, coList)).Should(Succeed(), "failed to list ClusterOperators.")

for _, co := range coList.Items {
Expect(framework.WaitForStatusAvailableOverLong(ctx, client, co.Name)).To(BeTrue(),
"Failed to wait for %s Cluster Operator to become available", co.Name)
}

By("removing the mitm-proxy")
framework.DeleteProxy(client)
})
})
39 changes: 29 additions & 10 deletions pkg/operators/machine-api-operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,10 @@ var _ = Describe(
Serial,
func() {
var (
gatherer *gatherer.StateGatherer
client runtimeclient.Client
ctx context.Context
gatherer *gatherer.StateGatherer
client runtimeclient.Client
ctx context.Context
isProxyJob bool
)

BeforeEach(func() {
Expand All @@ -288,11 +289,21 @@ var _ = Describe(
gatherer, err = framework.NewGatherer()
Expect(err).ToNot(HaveOccurred(), "Failed to load gatherer")

By("deploying an HTTP proxy")
framework.DeployProxy(client)
isProxyJob, err = framework.IsClusterProxyEnabled(ctx, client)
Expect(err).ToNot(HaveOccurred(), "Failed to check cluster proxy configuration")

By("configuring cluster-wide proxy")
framework.ConfigureClusterWideProxy(client)
if isProxyJob {
By("cluster-wide proxy is already configured on proxy cluster, skipping MITM proxy deployment")
} else {
By("deploying an HTTP proxy")
framework.DeployProxy(client)

By("configuring cluster-wide proxy")
framework.ConfigureClusterWideProxy(client)
}

By("waiting for machine-api-controllers to reflect proxy configuration")
framework.WaitForAllContainersProxyEnvVars(client, framework.MachineAPINamespace, maoManagedDeployment)
})

// Machines required for test: 1
Expand All @@ -312,14 +323,22 @@ var _ = Describe(
})

AfterEach(func() {
By("unconfiguring cluster-wide proxy")
framework.UnconfigureClusterWideProxy(client)

specReport := CurrentSpecReport()
if specReport.Failed() {
Expect(gatherer.WithSpecReport(specReport).GatherAll()).To(Succeed(), "Failed to GatherAll")
}

if isProxyJob {
By("cluster-wide proxy was pre-existing, skipping MITM proxy cleanup")
return
}

By("unconfiguring cluster-wide proxy")
framework.UnconfigureClusterWideProxy(client)

By("waiting for machine-api-controllers to reflect proxy removal")
framework.WaitForAllContainersNoProxyEnvVars(client, framework.MachineAPINamespace, maoManagedDeployment)

By("waiting for MAO, KAPI and KCM cluster operators to become available")

client, err := framework.LoadClient()
Expand Down