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

fix: Cluster-scoped Rollouts should be restricted to user-defined namepaces #95

Merged
merged 1 commit into from
Oct 3, 2024
Merged
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
1 change: 1 addition & 0 deletions api/v1alpha1/argorollouts_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ const (
RolloutManagerReasonErrorOccurred = "ErrorOccurred"
RolloutManagerReasonMultipleClusterScopedRolloutManager = "MultipleClusterScopedRolloutManager"
RolloutManagerReasonInvalidScoped = "InvalidRolloutManagerScope"
RolloutManagerReasonInvalidNamespace = "InvalidRolloutManagerNamespace"
)

type ResourceMetadata struct {
Expand Down
12 changes: 12 additions & 0 deletions controllers/argorollouts_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package rollouts
import (
"context"
"fmt"
"os"

rolloutsmanagerv1alpha1 "github.com/argoproj-labs/argo-rollouts-manager/api/v1alpha1"
"github.com/argoproj-labs/argo-rollouts-manager/tests/e2e/fixture/k8s"
Expand All @@ -28,6 +29,14 @@ var _ = Describe("RolloutManagerReconciler tests", func() {
BeforeEach(func() {
ctx = context.Background()
rm = makeTestRolloutManager()

By("Set Env variable.")
os.Setenv(ClusterScopedArgoRolloutsNamespaces, rm.Namespace)
jgwest marked this conversation as resolved.
Show resolved Hide resolved
})

AfterEach(func() {
By("Unset Env variable.")
os.Unsetenv(ClusterScopedArgoRolloutsNamespaces)
})

When("NAMESPACE_SCOPED_ARGO_ROLLOUTS environment variable is set to False.", func() {
Expand Down Expand Up @@ -201,6 +210,9 @@ var _ = Describe("RolloutManagerReconciler tests", func() {
rm2.Name = "test-rm"
rm2.Namespace = "test-ns"

By("Update Env variable.")
os.Setenv(ClusterScopedArgoRolloutsNamespaces, rm.Namespace+","+rm2.Namespace)

Expect(createNamespace(r, rm2.Namespace)).To(Succeed())
Expect(r.Client.Create(ctx, rm2)).ToNot(HaveOccurred())

Expand Down
3 changes: 3 additions & 0 deletions controllers/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,7 @@ const (
// NamespaceScopedArgoRolloutsController is an environment variable that can be used to configure scope of Argo Rollouts controller
// Set true to allow only namespace-scoped Argo Rollouts controller deployment and false for cluster-scoped
NamespaceScopedArgoRolloutsController = "NAMESPACE_SCOPED_ARGO_ROLLOUTS"

// ClusterScopedArgoRolloutsNamespaces is an environment variable that can be used to configure namespaces that are allowed to host cluster-scoped Argo Rollouts
ClusterScopedArgoRolloutsNamespaces = "CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES"
)
6 changes: 6 additions & 0 deletions controllers/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ func (r *RolloutManagerReconciler) reconcileRolloutsManager(ctx context.Context,
rr.condition = createCondition(err.Error(), rolloutsmanagerv1alpha1.RolloutManagerReasonInvalidScoped)
return *rr, nil
}

if invalidRolloutNamespace(err) {
rr.condition = createCondition(err.Error(), rolloutsmanagerv1alpha1.RolloutManagerReasonInvalidNamespace)
return *rr, nil
}

log.Error(err, "failed to validate RolloutManager's scope.")
return wrapCondition(createCondition(err.Error())), err
}
Expand Down
18 changes: 18 additions & 0 deletions controllers/resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package rollouts
import (
"context"
"fmt"
"os"

"github.com/argoproj-labs/argo-rollouts-manager/api/v1alpha1"
monitoringv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
Expand Down Expand Up @@ -809,6 +810,12 @@ var _ = Describe("Resource creation and cleanup tests", func() {
})

Context("Resource Cleanup test", func() {

AfterEach(func() {
By("Unset Env variable.")
os.Unsetenv(ClusterScopedArgoRolloutsNamespaces)
})

a := makeTestRolloutManager()
tt := []struct {
name string
Expand Down Expand Up @@ -875,6 +882,9 @@ var _ = Describe("Resource creation and cleanup tests", func() {
When(test.name, func() {
It("Cleans up all resources created for RolloutManager", func() {

By("Set Env variable.")
os.Setenv(ClusterScopedArgoRolloutsNamespaces, a.Namespace)
jgwest marked this conversation as resolved.
Show resolved Hide resolved

ctx := context.Background()
req := reconcile.Request{
NamespacedName: types.NamespacedName{
Expand Down Expand Up @@ -919,6 +929,14 @@ var _ = Describe("Resource creation and cleanup tests", func() {
Namespace: a.Namespace,
},
}

By("Set Env variable.")
os.Setenv(ClusterScopedArgoRolloutsNamespaces, a.Namespace)
})

AfterEach(func() {
By("Unset Env variable.")
os.Unsetenv(ClusterScopedArgoRolloutsNamespaces)
})

It("Verify whether RolloutManager creating ServiceMonitor", func() {
Expand Down
46 changes: 42 additions & 4 deletions controllers/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ import (
)

const (
UnsupportedRolloutManagerConfiguration = "when there exists a cluster-scoped RolloutManager on the cluster, there may not exist another: only a single cluster-scoped RolloutManager is supported"
UnsupportedRolloutManagerClusterScoped = "when Subscription has environment variable NAMESPACE_SCOPED_ARGO_ROLLOUTS set to True, there may not exist any cluster-scoped RolloutManagers: in this case, only namespace-scoped RolloutManager resources are supported"
UnsupportedRolloutManagerNamespaceScoped = "when Subscription has environment variable NAMESPACE_SCOPED_ARGO_ROLLOUTS set to False, there may not exist any namespace-scoped RolloutManagers: only a single cluster-scoped RolloutManager is supported"
UnsupportedRolloutManagerConfiguration = "when there exists a cluster-scoped RolloutManager on the cluster, there may not exist another: only a single cluster-scoped RolloutManager is supported"
UnsupportedRolloutManagerClusterScoped = "when Subscription has environment variable NAMESPACE_SCOPED_ARGO_ROLLOUTS set to True, there may not exist any cluster-scoped RolloutManagers: in this case, only namespace-scoped RolloutManager resources are supported"
UnsupportedRolloutManagerNamespaceScoped = "when Subscription has environment variable NAMESPACE_SCOPED_ARGO_ROLLOUTS set to False, there may not exist any namespace-scoped RolloutManagers: only a single cluster-scoped RolloutManager is supported"
UnsupportedRolloutManagerClusterScopedNamespace = "Namespace is not specified in CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES environment variable of Subscription resource. If you wish to install a cluster-scoped Argo Rollouts instance outside the default namespace, ensure it is defined in CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES"
)

// pluginItem is a clone of PluginItem from "github.com/argoproj/argo-rollouts/utils/plugin/types"
Expand Down Expand Up @@ -235,11 +236,44 @@ func validateRolloutsScope(cr rolloutsmanagerv1alpha1.RolloutManager, namespaceS
}, errors.New(UnsupportedRolloutManagerNamespaceScoped)
}

// allow only cluster-scoped RolloutManager
// if cluster-scoped RolloutManager being reconciled, is not specified in CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES environment variable of Subscription resource,
// then don't allow it.
if !allowedClusterScopedNamespace(cr) {

phaseFailure := rolloutsmanagerv1alpha1.PhaseFailure

return &reconcileStatusResult{
rolloutController: &phaseFailure,
phase: &phaseFailure,
}, errors.New(UnsupportedRolloutManagerClusterScopedNamespace)
}

// allow cluster-scoped Rollouts for namespaces specified in CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES environment variable of Subscription resource
return nil, nil
}
}

// allowedClusterScopedNamespace will check that current namespace is allowed to host cluster-scoped Argo Rollouts.
func allowedClusterScopedNamespace(cr rolloutsmanagerv1alpha1.RolloutManager) bool {
clusterConfigNamespaces := splitList(os.Getenv(ClusterScopedArgoRolloutsNamespaces))
if len(clusterConfigNamespaces) > 0 {
for _, n := range clusterConfigNamespaces {
if n == cr.Namespace {
return true
}
}
}
return false
}

func splitList(s string) []string {
elems := strings.Split(s, ",")
for i := range elems {
elems[i] = strings.TrimSpace(elems[i])
}
return elems
}

// checkForExistingRolloutManager will return error if more than one cluster-scoped RolloutManagers are created.
// because only one cluster-scoped or all namespace-scoped RolloutManagers are supported.
func checkForExistingRolloutManager(ctx context.Context, k8sClient client.Client, cr rolloutsmanagerv1alpha1.RolloutManager) (*reconcileStatusResult, error) {
Expand Down Expand Up @@ -290,6 +324,10 @@ func invalidRolloutScope(err error) bool {
err.Error() == UnsupportedRolloutManagerNamespaceScoped
}

func invalidRolloutNamespace(err error) bool {
return err.Error() == UnsupportedRolloutManagerClusterScopedNamespace
}

// updateStatusConditionOfRolloutManager calls Set Condition of RolloutManager status
func updateStatusConditionOfRolloutManager(ctx context.Context, rr reconcileStatusResult, rm *rolloutsmanagerv1alpha1.RolloutManager, k8sClient client.Client, log logr.Logger) error {

Expand Down
29 changes: 28 additions & 1 deletion controllers/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package rollouts

import (
"context"
"os"

rolloutsmanagerv1alpha1 "github.com/argoproj-labs/argo-rollouts-manager/api/v1alpha1"
monitoringv1 "github.com/coreos/prometheus-operator/pkg/apis/monitoring/v1"
Expand Down Expand Up @@ -360,9 +361,19 @@ var _ = Describe("validateRolloutsScope tests", func() {

When("NAMESPACE_SCOPED_ARGO_ROLLOUTS environment variable is set to False.", func() {

BeforeEach(func() {
By("Set Env variable.")
os.Setenv(ClusterScopedArgoRolloutsNamespaces, rolloutsManager.Namespace)
})

AfterEach(func() {
By("Unset Env variable.")
os.Unsetenv(ClusterScopedArgoRolloutsNamespaces)
})

namespaceScopedArgoRolloutsController := false

It("should not return error, if cluster-scoped RolloutManager is created.", func() {
It("should not return error, if cluster-scoped RolloutManager is created in a namespace specified in env variable.", func() {
jgwest marked this conversation as resolved.
Show resolved Hide resolved

By("Create cluster-scoped RolloutManager.")
Expect(k8sClient.Create(ctx, &rolloutsManager)).To(Succeed())
Expand All @@ -373,6 +384,22 @@ var _ = Describe("validateRolloutsScope tests", func() {
Expect(rr).To(BeNil())
})

It("should return error, if cluster-scoped RolloutManager is created in a namespace which is not specified in env variable.", func() {

By("Unset Env variable.")
os.Unsetenv(ClusterScopedArgoRolloutsNamespaces)

By("Create cluster-scoped RolloutManager.")
Expect(k8sClient.Create(ctx, &rolloutsManager)).To(Succeed())

By("Verify there is no error returned.")
rr, err := validateRolloutsScope(rolloutsManager, namespaceScopedArgoRolloutsController)
Expect(err).To(HaveOccurred())
Expect(invalidRolloutNamespace(err)).To(BeTrue())
Expect(*rr.phase).To(Equal(rolloutsmanagerv1alpha1.PhaseFailure))
Expect(*rr.rolloutController).To(Equal(rolloutsmanagerv1alpha1.PhaseFailure))
})

It("should return error, if namespace-scoped RolloutManager is created.", func() {

By("Create namespace-scoped RolloutManager.")
Expand Down
46 changes: 46 additions & 0 deletions docs/usage/getting_started.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,49 @@ kubectl get all

If you would like to understand the siginificance of each rollout controller resource created by the operator, please go through the official rollouts controller [docs](https://argo-rollouts.readthedocs.io/en/stable/).




## Namespace Scoped Rollouts Instance

A namespace-scoped Rollouts instance can manage Rollouts resources of same namespace it is deployed into. To deploy a namespace-scoped Rollouts instance set `spec.namespaceScoped` field to `true`.

```yml
apiVersion: argoproj.io/v1alpha1
kind: RolloutManager
metadata:
name: argo-rollout
spec:
namespaceScoped: true
```


## Cluster Scoped Rollouts Instance

A cluster-scoped Rollouts instance can manage Rollouts resources from other namespaces as well. To install a cluster-scoped Rollouts instance first you need to add `NAMESPACE_SCOPED_ARGO_ROLLOUTS` and `CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES` environment variables in subscription resource. If `NAMESPACE_SCOPED_ARGO_ROLLOUTS` is set to `false` then only you are allowed to create a cluster-scoped instance and then you need to provide list of namespaces that are allowed host a cluster-scoped Rollouts instance via `CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES` environment variable.

```yml
apiVersion: operators.coreos.com/v1alpha1
kind: Subscription
metadata:
name: argo-operator
spec:
config:
env:
- name: NAMESPACE_SCOPED_ARGO_ROLLOUTS
value: 'false'
- name: CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES
value: <list of namespaces of cluster-scoped Argo CD instances>
(...)
```

Now set `spec.namespaceScoped` field to `false` to create a Rollouts instance.

```yml
apiVersion: argoproj.io/v1alpha1
kind: RolloutManager
metadata:
name: argo-rollout
spec:
namespaceScoped: false
```
4 changes: 4 additions & 0 deletions hack/run-upstream-argo-rollouts-e2e-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ if [ -z "$SKIP_RUN_STEP" ]; then
set +e

rm -f /tmp/e2e-operator-run.log || true

# Set namespaces used for cluster-scoped e2e tests
export CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES="argo-rollouts"

go run ./cmd/main.go 2>&1 | tee /tmp/e2e-operator-run.log &

set -e
Expand Down
3 changes: 3 additions & 0 deletions hack/start-rollouts-manager-for-e2e-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ set -ex

make install generate fmt vet

# Set namespaces used for cluster-scoped e2e tests
export CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES="argo-rollouts,test-rom-ns-1,rom-ns-1"

jgwest marked this conversation as resolved.
Show resolved Hide resolved
if [ "$RUN_IN_BACKGROUND" == "true" ]; then
go run ./cmd/main.go 2>&1 | tee /tmp/e2e-operator-run.log &
else
Expand Down
25 changes: 25 additions & 0 deletions tests/e2e/cluster-scoped/cluster_scoped_rollouts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,31 @@ var _ = Describe("Cluster-scoped RolloutManager tests", func() {
utils.ValidateArgoRolloutsResources(ctx, k8sClient, nsName2, testServiceNodePort_31002, testServiceNodePort_32002)
})

/*
In this test a cluster-scoped RolloutManager is created in one namespace, but namespace is not specified in CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES env variable, hence operator should not allow to create Rollouts instance.
*/
It("Namespace is not specified in CLUSTER_SCOPED_ARGO_ROLLOUTS_NAMESPACES, then cluster-scoped RolloutManager should not be allowed.", func() {

nsName := "test-ns"
By("Create namespace.")
Expect(utils.CreateNamespace(ctx, k8sClient, nsName)).To(Succeed())

rolloutsManager, err := utils.CreateRolloutManager(ctx, k8sClient, "test-rollouts-manager-1", nsName, false)
Expect(err).ToNot(HaveOccurred())

By("Verify that RolloutManager is successfully created.")
Eventually(rolloutsManager, "1m", "5s").Should(rmFixture.HavePhase(rmv1alpha1.PhaseFailure))

By("Verify that Status.Condition is now having error message.")
Eventually(rolloutsManager, "1m", "1s").Should(rmFixture.HaveCondition(
metav1.Condition{
Type: rmv1alpha1.RolloutManagerConditionType,
Status: metav1.ConditionFalse,
Reason: rmv1alpha1.RolloutManagerReasonInvalidNamespace,
Message: controllers.UnsupportedRolloutManagerClusterScopedNamespace,
}))
})

It("After creating 2 cluster-scoped RolloutManager in a namespace, delete 1st RolloutManager and verify it removes the Failed status of 2nd RolloutManager", func() {
By("1st RM: Create cluster-scoped RolloutManager in a namespace.")
rolloutsManagerCl, err := utils.CreateRolloutManager(ctx, k8sClient, "test-rollouts-manager-1", fixture.TestE2ENamespace, false)
Expand Down
Loading