Skip to content

Commit

Permalink
fix: Cluster-scoped Rollouts should be restricted to user-defined nam…
Browse files Browse the repository at this point in the history
…espace

Signed-off-by: Jayendra Parsai <[email protected]>
  • Loading branch information
Jayendra Parsai authored and jgwest committed Oct 4, 2024
1 parent b3e573f commit b7bce6c
Show file tree
Hide file tree
Showing 11 changed files with 273 additions and 6 deletions.
1 change: 1 addition & 0 deletions api/v1alpha1/argorollouts_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,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)
})

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 @@ -187,6 +188,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 @@ -253,6 +260,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)

ctx := context.Background()
req := reconcile.Request{
NamespacedName: types.NamespacedName{
Expand Down Expand Up @@ -297,6 +307,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 @@ -213,11 +214,44 @@ func validateRolloutsScope(cr rolloutsmanagerv1alpha1.RolloutManager, namespaceS
}, fmt.Errorf(UnsupportedRolloutManagerNamespaceScoped)

Check failure on line 214 in controllers/utils.go

View workflow job for this annotation

GitHub Actions / Run golangci-lint and gosec

SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck)
}

// 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 @@ -268,6 +302,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 @@ -330,9 +331,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() {

By("Create cluster-scoped RolloutManager.")
Expect(k8sClient.Create(ctx, &rolloutsManager)).To(Succeed())
Expand All @@ -343,6 +354,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
```
6 changes: 5 additions & 1 deletion hack/run-upstream-argo-rollouts-e2e-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,11 @@ if [ -z "$SKIP_RUN_STEP" ]; then
set +e

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

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

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

set -e
fi
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"

if [ "$RUN_IN_BACKGROUND" == "true" ]; then
go run ./main.go 2>&1 | tee /tmp/e2e-operator-run.log &
else
Expand Down
Loading

0 comments on commit b7bce6c

Please sign in to comment.