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

Add tls cert mount appset controller #985

Merged
Show file tree
Hide file tree
Changes from 5 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
3 changes: 3 additions & 0 deletions api/v1beta1/argocd_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ type ArgoCDApplicationSet struct {
LogLevel string `json:"logLevel,omitempty"`

WebhookServer WebhookServerSpec `json:"webhookServer,omitempty"`

// SCMRootCAPath is the path where the Gitlab SCM Provider's TLS certificate is mounted on the ApplicationSet Controller.
SCMRootCAPath string `json:"scmRootCaPath,omitempty"`
}

// ArgoCDCASpec defines the CA options for ArgCD.
Expand Down
4 changes: 4 additions & 0 deletions bundle/manifests/argoproj.io_argocds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6653,6 +6653,10 @@ spec:
to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
type: object
type: object
scmRootCaPath:
description: SCMRootCAPath is the path where the Gitlab SCM Provider's
TLS certificate is mounted on the ApplicationSet Controller.
type: string
version:
description: Version is the Argo CD ApplicationSet image tag.
(optional)
Expand Down
3 changes: 3 additions & 0 deletions common/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ const (
// ArgoCDTLSCertsConfigMapName is the upstream hard-coded TLS certificate data ConfigMap name.
ArgoCDTLSCertsConfigMapName = "argocd-tls-certs-cm"

// ArgoCDAppSetGitlabSCMTLSCertsConfigMapName is the hard-coded ApplicationSet Gitlab SCM TLS certificate data ConfigMap name.
ArgoCDAppSetGitlabSCMTLSCertsConfigMapName = "argocd-appset-gitlab-scm-tls-certs-cm"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any specific reason for going with config map and not a secret ?

Copy link
Collaborator Author

@ishitasequeira ishitasequeira Sep 1, 2023

Choose a reason for hiding this comment

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

No specific reason, followed what we have used for TLS certs.


// ArgoCDRedisServerTLSSecretName is the name of the TLS secret for the redis-server
ArgoCDRedisServerTLSSecretName = "argocd-operator-redis-tls"

Expand Down
4 changes: 4 additions & 0 deletions config/crd/bases/argoproj.io_argocds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6644,6 +6644,10 @@ spec:
to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
type: object
type: object
scmRootCaPath:
description: SCMRootCAPath is the path where the Gitlab SCM Provider's
TLS certificate is mounted on the ApplicationSet Controller.
type: string
version:
description: Version is the Argo CD ApplicationSet image tag.
(optional)
Expand Down
35 changes: 32 additions & 3 deletions controllers/argocd/applicationset.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ func getArgoApplicationSetCommand(cr *argoproj.ArgoCD) []string {
cmd = append(cmd, "--loglevel")
cmd = append(cmd, getLogLevel(cr.Spec.ApplicationSet.LogLevel))

if cr.Spec.ApplicationSet.SCMRootCAPath != "" {
cmd = append(cmd, "--scm-root-ca-path")
cmd = append(cmd, cr.Spec.ApplicationSet.SCMRootCAPath)
}

// ApplicationSet command arguments provided by the user
extraArgs := cr.Spec.ApplicationSet.ExtraCommandArgs
err := isMergable(extraArgs, cmd)
Expand Down Expand Up @@ -144,9 +149,26 @@ func (r *ReconcileArgoCD) reconcileApplicationSetDeployment(cr *argoproj.ArgoCD,
},
},
}
addSCMGitlabVolumeMount := false
if cr.Spec.ApplicationSet.SCMRootCAPath != "" {
Copy link
Collaborator

@anandf anandf Sep 1, 2023

Choose a reason for hiding this comment

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

Why should we get the CA mount path from the user, if the operator is going to mount it from a well defined CM.
Alternatively, Can the operator create an empty config map argocd-appset-gitlab-scm-tls-certs-cm and mount it in a well defined path like /app/tls/cert and if at all the user wants to use this feature, all he has to do is to fill Certificate data in config map argocd-appset-gitlab-scm-tls-certs-cm. With this approach, it would be easy for the operator to manage the Volume and VolumeMounts as its no longer conditonal. Since the mount path is well defined, there won't be any path restrictions that user has to be aware of when providing the value for scmRootCAPath.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The alternate approach, would be similar to how TLS certs are handled in ArgoCD Server.

oc get deploy -n openshift-gitops openshift-gitops-server -o yaml
....
- mountPath: /app/config/tls
          name: tls-certs
....
volumes:
      - configMap:
          defaultMode: 420
          name: argocd-tls-certs-cm
        name: tls-certs
oc get cm -n openshift-gitops argocd-tls-certs-cm -o yaml

apiVersion: v1
kind: ConfigMap
metadata:
  name: argocd-tls-certs-cm
  namespace: openshift-gitops

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We would not want to create an empty CM as the requests for Gitlab client would use these certs only if the parameter --scm-root-ca-path is set on the ApplicationController pod. And, we would want to set this path only if user has provided the ConfigMap.

I do like the thought of hard-coding the path of the cert, but I am wondering if there would be any use case where user would want to specify the path. I am open to hard-coding the path for volume mount.

Copy link
Collaborator

@anandf anandf Sep 1, 2023

Choose a reason for hiding this comment

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

In that case can we check for the existence of a config map with name argocd-appset-gitlab-scm-tls-certs-cm or with a label selector and if it exists, then create a volume and mount it using a volumemount at well defined hardcoded path.

Copy link
Collaborator Author

@ishitasequeira ishitasequeira Sep 1, 2023

Choose a reason for hiding this comment

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

Yes, we can do that. I will update the code. Do you have a path in mind?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The one you had suggested in the example, /app/tls/cert looks good to me. If we want to give some kind of user configurable option, to explicitly enable this feature, then we can consider taking in the config map name in the ArgoCD spec and keep the mount path hard coded to /app/tls/cert

Copy link
Collaborator Author

@ishitasequeira ishitasequeira Sep 5, 2023

Choose a reason for hiding this comment

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

I updated to use /app/tls/scm/cert as the path. I also took ConfigMap Name as the input from users, not restricting them to a specific ConfigMap.

cm := newConfigMapWithName(getCAConfigMapName(cr), cr)
if argoutil.IsObjectFound(r.Client, cr.Namespace, common.ArgoCDAppSetGitlabSCMTLSCertsConfigMapName, cm) {
addSCMGitlabVolumeMount = true
podSpec.Volumes = append(podSpec.Volumes, corev1.Volume{
Name: "appset-gitlab-scm-tls-cert",
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: common.ArgoCDAppSetGitlabSCMTLSCertsConfigMapName,
},
},
},
})
}
}

podSpec.Containers = []corev1.Container{
applicationSetContainer(cr),
applicationSetContainer(cr, addSCMGitlabVolumeMount),
}
AddSeccompProfileForOpenShift(r.Client, podSpec)

Expand Down Expand Up @@ -185,7 +207,7 @@ func (r *ReconcileArgoCD) reconcileApplicationSetDeployment(cr *argoproj.ArgoCD,

}

func applicationSetContainer(cr *argoproj.ArgoCD) corev1.Container {
func applicationSetContainer(cr *argoproj.ArgoCD, addSCMGitlabVolumeMount bool) corev1.Container {
// Global proxy env vars go first
appSetEnv := []corev1.EnvVar{{
Name: "NAMESPACE",
Expand All @@ -202,7 +224,7 @@ func applicationSetContainer(cr *argoproj.ArgoCD) corev1.Container {
// Environment specified in the CR take precedence over everything else
appSetEnv = argoutil.EnvMerge(appSetEnv, proxyEnvVars(), false)

return corev1.Container{
container := corev1.Container{
Command: getArgoApplicationSetCommand(cr),
Env: appSetEnv,
Image: getApplicationSetContainerImage(cr),
Expand Down Expand Up @@ -252,6 +274,13 @@ func applicationSetContainer(cr *argoproj.ArgoCD) corev1.Container {
RunAsNonRoot: boolPtr(true),
},
}
if addSCMGitlabVolumeMount {
container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
Name: "appset-gitlab-scm-tls-cert",
MountPath: cr.Spec.ApplicationSet.SCMRootCAPath,
})
}
return container
}

func (r *ReconcileArgoCD) reconcileApplicationSetServiceAccount(cr *argoproj.ArgoCD) (*corev1.ServiceAccount, error) {
Expand Down
36 changes: 30 additions & 6 deletions controllers/argocd/applicationset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"

argoproj "github.com/argoproj-labs/argocd-operator/api/v1beta1"
Expand Down Expand Up @@ -92,14 +93,14 @@ func TestReconcileApplicationSet_CreateDeployments(t *testing.T) {
deployment))

// Ensure the created Deployment has the expected properties
checkExpectedDeploymentValues(t, deployment, &sa, a)
checkExpectedDeploymentValues(t, r, deployment, &sa, a)
}

func checkExpectedDeploymentValues(t *testing.T, deployment *appsv1.Deployment, sa *corev1.ServiceAccount, a *argoproj.ArgoCD) {
func checkExpectedDeploymentValues(t *testing.T, r *ReconcileArgoCD, deployment *appsv1.Deployment, sa *corev1.ServiceAccount, a *argoproj.ArgoCD) {
assert.Equal(t, deployment.Spec.Template.Spec.ServiceAccountName, sa.ObjectMeta.Name)
appsetAssertExpectedLabels(t, &deployment.ObjectMeta)

want := []corev1.Container{applicationSetContainer(a)}
want := []corev1.Container{applicationSetContainer(a, false)}

if diff := cmp.Diff(want, deployment.Spec.Template.Spec.Containers); diff != "" {
t.Fatalf("failed to reconcile applicationset-controller deployment containers:\n%s", diff)
Expand Down Expand Up @@ -150,6 +151,19 @@ func checkExpectedDeploymentValues(t *testing.T, deployment *appsv1.Deployment,
},
}

if a.Spec.ApplicationSet.SCMRootCAPath != "" && argoutil.IsObjectFound(r.Client, a.Namespace, common.ArgoCDAppSetGitlabSCMTLSCertsConfigMapName, a) {
volumes = append(volumes, corev1.Volume{
Name: "appset-gitlab-scm-tls-cert",
VolumeSource: corev1.VolumeSource{
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{
Name: common.ArgoCDAppSetGitlabSCMTLSCertsConfigMapName,
},
},
},
})
}

if diff := cmp.Diff(volumes, deployment.Spec.Template.Spec.Volumes); diff != "" {
t.Fatalf("failed to reconcile applicationset-controller deployment volumes:\n%s", diff)
}
Expand Down Expand Up @@ -261,7 +275,7 @@ func TestReconcileApplicationSet_UpdateExistingDeployments(t *testing.T) {
deployment))

// Ensure the updated Deployment has the expected properties
checkExpectedDeploymentValues(t, deployment, &sa, a)
checkExpectedDeploymentValues(t, r, deployment, &sa, a)

}

Expand All @@ -287,7 +301,7 @@ func TestReconcileApplicationSet_Deployments_resourceRequirements(t *testing.T)
assert.Equal(t, deployment.Spec.Template.Spec.ServiceAccountName, sa.ObjectMeta.Name)
appsetAssertExpectedLabels(t, &deployment.ObjectMeta)

containerWant := []corev1.Container{applicationSetContainer(a)}
containerWant := []corev1.Container{applicationSetContainer(a, false)}

if diff := cmp.Diff(containerWant, deployment.Spec.Template.Spec.Containers); diff != "" {
t.Fatalf("failed to reconcile argocd-server deployment:\n%s", diff)
Expand Down Expand Up @@ -346,6 +360,14 @@ func TestReconcileApplicationSet_Deployments_SpecOverride(t *testing.T) {
envVars: map[string]string{common.ArgoCDImageEnvName: "custom-env-image"},
expectedContainerImage: "custom-image:custom-version",
},
{
name: "ensure scm tls cert mount is present",
appSetField: &argoproj.ArgoCDApplicationSet{
SCMRootCAPath: "testPath",
},
envVars: map[string]string{common.ArgoCDImageEnvName: "custom-env-image"},
expectedContainerImage: "custom-env-image",
},
}

for _, test := range tests {
Expand All @@ -357,6 +379,8 @@ func TestReconcileApplicationSet_Deployments_SpecOverride(t *testing.T) {

a := makeTestArgoCD()
r := makeTestReconciler(t, a)
cm := newConfigMapWithName(getCAConfigMapName(a), a)
r.Client.Create(context.Background(), cm, &client.CreateOptions{})

a.Spec.ApplicationSet = test.appSetField

Expand All @@ -374,7 +398,7 @@ func TestReconcileApplicationSet_Deployments_SpecOverride(t *testing.T) {

specImage := deployment.Spec.Template.Spec.Containers[0].Image
assert.Equal(t, test.expectedContainerImage, specImage)

checkExpectedDeploymentValues(t, r, deployment, &sa, a)
})
}

Expand Down
2 changes: 1 addition & 1 deletion controllers/argocd/argocd_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,6 @@ func (r *ReconcileArgoCD) Reconcile(ctx context.Context, request ctrl.Request) (
// SetupWithManager sets up the controller with the Manager.
func (r *ReconcileArgoCD) SetupWithManager(mgr ctrl.Manager) error {
bldr := ctrl.NewControllerManagedBy(mgr)
r.setResourceWatches(bldr, r.clusterResourceMapper, r.tlsSecretMapper, r.namespaceResourceMapper, r.clusterSecretResourceMapper)
r.setResourceWatches(bldr, r.clusterResourceMapper, r.tlsSecretMapper, r.namespaceResourceMapper, r.clusterSecretResourceMapper, r.applicationSetSCMTLSConfigMapMapper)
return bldr.Complete(r)
}
28 changes: 28 additions & 0 deletions controllers/argocd/custommapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,31 @@ func (r *ReconcileArgoCD) clusterSecretResourceMapper(o client.Object) []reconci

return result
}

// applicationSetSCMTLSConfigMapMapper maps a watch event on a configmap with name "argocd-appset-gitlab-scm-tls-certs-cm",
// back to the ArgoCD object that we want to reconcile.
func (r *ReconcileArgoCD) applicationSetSCMTLSConfigMapMapper(o client.Object) []reconcile.Request {
var result = []reconcile.Request{}

if o.GetName() == common.ArgoCDAppSetGitlabSCMTLSCertsConfigMapName {
argocds := &argoproj.ArgoCDList{}
if err := r.Client.List(context.TODO(), argocds, &client.ListOptions{Namespace: o.GetNamespace()}); err != nil {
return result
}

if len(argocds.Items) != 1 {
return result
}

argocd := argocds.Items[0]
namespacedName := client.ObjectKey{
Name: argocd.Name,
Namespace: argocd.Namespace,
}
result = []reconcile.Request{
{NamespacedName: namespacedName},
}
}

return result
}
8 changes: 7 additions & 1 deletion controllers/argocd/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,7 @@ func removeString(slice []string, s string) []string {
}

// setResourceWatches will register Watches for each of the supported Resources.
func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResourceMapper, tlsSecretMapper, namespaceResourceMapper, clusterSecretResourceMapper handler.MapFunc) *builder.Builder {
func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResourceMapper, tlsSecretMapper, namespaceResourceMapper, clusterSecretResourceMapper, applicationSetGitlabSCMTLSConfigMapMapper handler.MapFunc) *builder.Builder {

deploymentConfigPred := predicate.Funcs{
UpdateFunc: func(e event.UpdateEvent) bool {
Expand Down Expand Up @@ -1046,12 +1046,18 @@ func (r *ReconcileArgoCD) setResourceWatches(bldr *builder.Builder, clusterResou

clusterSecretResourceHandler := handler.EnqueueRequestsFromMapFunc(clusterSecretResourceMapper)

appSetGitlabSCMTLSConfigMapHandler := handler.EnqueueRequestsFromMapFunc(applicationSetGitlabSCMTLSConfigMapMapper)

tlsSecretHandler := handler.EnqueueRequestsFromMapFunc(tlsSecretMapper)

bldr.Watches(&source.Kind{Type: &v1.ClusterRoleBinding{}}, clusterResourceHandler)

bldr.Watches(&source.Kind{Type: &v1.ClusterRole{}}, clusterResourceHandler)

bldr.Watches(&source.Kind{Type: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{
Name: common.ArgoCDAppSetGitlabSCMTLSCertsConfigMapName,
}}}, appSetGitlabSCMTLSConfigMapHandler)

// Watch for secrets of type TLS that might be created by external processes
bldr.Watches(&source.Kind{Type: &corev1.Secret{Type: corev1.SecretTypeTLS}}, tlsSecretHandler)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6653,6 +6653,10 @@ spec:
to an implementation-defined value. More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/'
type: object
type: object
scmRootCaPath:
description: SCMRootCAPath is the path where the Gitlab SCM Provider's
TLS certificate is mounted on the ApplicationSet Controller.
type: string
version:
description: Version is the Argo CD ApplicationSet image tag.
(optional)
Expand Down
19 changes: 19 additions & 0 deletions docs/reference/argocd.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ Resources | [Empty] | The container compute resources.
LogLevel | info | The log level to be used by the ArgoCD Application Controller component. Valid options are debug, info, error, and warn.
LogFormat | text | The log format to be used by the ArgoCD Application Controller component. Valid options are text or json.
ParallelismLimit | 10 | The kubectl parallelism limit to set for the controller (`--kubectl-parallelism-limit` flag)
SCMRootCaPath (#add-tls-certificate-for-gitlab-scm-provider-to-applicationsets-controller) | [Empty] | The path where the Gitlab SCM Provider's TLS certificate is mounted on the ApplicationSet Controller. The TLS certificate is picked from a configMap `"argocd-appset-gitlab-scm-tls-certs-cm"` and mounted on the applicationset-controller as a volume mount.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct name to SCMRootCAPath


### ApplicationSet Controller Example

Expand Down Expand Up @@ -119,6 +120,24 @@ spec:
- bar
```

### Add Self signed TLS Certificate for Gitlab SCM Provider to ApplicationSets Controller

ApplicationSetController added a new option `--scm-root-ca-path` and expects the self-signed TLS certificate to be mounted on the path specified and to be used for Gitlab SCM Provider and Gitlab Pull Request Provider. To set this option, you can set `spec.applicationSet.scmRootCaPath` in ArgoCD CR. The operator expects the TLS certificate to be stored in a ConfigMap named `argocd-appset-gitlab-scm-tls-certs-cm`. When the parameter `spec.applicationSet.scmRootCaPath` is set in ArgoCD CR, the operator checks for ConfigMap named `argocd-appset-gitlab-scm-tls-certs-cm` in the same namespace as the ArgoCD instance and mounts the Certificate stored in ConfigMap to ApplicationSet Controller pods at the path specified by `spec.applicationSet.scmRootCaPath`.

Below example shows how a user can add scmRootCaPath to the ApplicationSet controller.
```yaml
apiVersion: argoproj.io/v1alpha1
kind: ArgoCD
metadata:
name: example-argocd
labels:
example: applicationset
spec:
applicationSet:
scmRootCaPath: /path/for/tls/certificate/mount
```


## Config Management Plugins

Configuration to add a config management plugin. This property maps directly to the `configManagementPlugins` field in the `argocd-cm` ConfigMap.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
apiVersion: kuttl.dev/v1beta1
kind: TestAssert
timeout: 120
---
apiVersion: argoproj.io/v1beta1
kind: ArgoCD
metadata:
name: argocd
namespace: test-1-32-appsets-scm-tls-mount
spec:
applicationSet:
scmRootCaPath: /app/tls/cert
status:
phase: Available
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: argocd-applicationset-controller
namespace: test-1-32-appsets-scm-tls-mount
labels:
app.kubernetes.io/component: controller
app.kubernetes.io/managed-by: argocd
app.kubernetes.io/name: argocd-applicationset-controller
app.kubernetes.io/part-of: argocd-applicationset
spec:
selector:
matchLabels:
app.kubernetes.io/name: argocd-applicationset-controller
template:
spec:
containers:
- command:
- entrypoint.sh
- argocd-applicationset-controller
- --argocd-repo-server
- argocd-repo-server.test-1-32-appsets-scm-tls-mount.svc.cluster.local:8081
- --loglevel
- info
- --scm-root-ca-path
- /app/tls/cert
volumeMounts:
- mountPath: /app/config/ssh
name: ssh-known-hosts
- mountPath: /app/config/tls
name: tls-certs
- mountPath: /app/config/gpg/source
name: gpg-keys
- mountPath: /app/config/gpg/keys
name: gpg-keyring
- mountPath: /tmp
name: tmp
- mountPath: /app/tls/cert
name: appset-gitlab-scm-tls-cert
volumes:
- configMap:
defaultMode: 420
name: argocd-ssh-known-hosts-cm
name: ssh-known-hosts
- configMap:
defaultMode: 420
name: argocd-tls-certs-cm
name: tls-certs
- configMap:
defaultMode: 420
name: argocd-gpg-keys-cm
name: gpg-keys
- emptyDir: {}
name: gpg-keyring
- emptyDir: {}
name: tmp
- configMap:
defaultMode: 420
name: argocd-appset-gitlab-scm-tls-certs-cm
name: appset-gitlab-scm-tls-cert
Loading
Loading