|
| 1 | +# GitOpsDeploymentManagedEnvironment API: ‘cluster add’ behaviour |
| 2 | + |
| 3 | +### Written by |
| 4 | +- Jonathan West (@jgwest) |
| 5 | +- Originally written in December 7, 2022 |
| 6 | + |
| 7 | +New AppStudio requirements (primarily driven by post-KCP shift) are requiring us to rethink the behaviour of one of the Core GitOps Service APIs, **GitOpsDeploymentManagedEnvironment**. |
| 8 | + |
| 9 | +Primarily this is about how to add new clusters (as deployment targets) to the GitOps Service. When a user gives us credentials for the environment, should we either: |
| 10 | + |
| 11 | +* **A)** Create a new ServiceAccount on the cluster using the given credentials (e.g. copy the behaviour of the 'argocd cluster add' CLI command) |
| 12 | +* Or, **B)** Use the credentials as is: give the provided API URL/token credential directly to Argo CD. |
| 13 | + |
| 14 | +## Final decision |
| 15 | + |
| 16 | +The final outcome of this document, as of March 7th, 2023, was to follow option A: supporting both the old and new behaviour. |
| 17 | + |
| 18 | +Which behaviour to use would be designed with the proposed field ‘ createNewServiceAccount=true/false’, as seen under option A below. |
| 19 | + |
| 20 | +## Current behaviour |
| 21 | + |
| 22 | +**TL;DR**: The GitOps Service follows the same behaviour as the '*argocd cluster add*' CLI command: it creates a new cluster-scoped ServiceAccount on the target cluster using the credentials provided by the user. |
| 23 | + |
| 24 | +When the user wants to deploy to a target environment, they give us a ManagedEnvironment and Secret for that Environment, like so: |
| 25 | + |
| 26 | +**ManagedEnv:** |
| 27 | + |
| 28 | +```yaml |
| 29 | +kind: GitOpsDeploymentManagedEnvironment |
| 30 | +spec: |
| 31 | + apiURL: (api url containing credentials to use for deployment) |
| 32 | + credentialsSecret: (secret containing credentials to use for deployment; these will be provided as is to Argo CD) |
| 33 | +``` |
| 34 | +
|
| 35 | +**Secret:** |
| 36 | +
|
| 37 | +
|
| 38 | +``` yaml |
| 39 | +# The GitOpsDeploymentManagedEnvironment references a Secret, containing the connection information |
| 40 | +# - Kubeconfig credentials for the target cluster (as a Secret) |
| 41 | +apiVersion: v1 |
| 42 | +kind: Secret |
| 43 | +metadata: |
| 44 | + name: my-managed-environment-secret |
| 45 | + namespace: jane |
| 46 | +type: managed-gitops.redhat.com/managed-environment |
| 47 | +data: |
| 48 | + # Note: This would be base 64 when stored on the cluster |
| 49 | + kubeconfig: | |
| 50 | + apiVersion: v1 |
| 51 | + clusters: |
| 52 | + - cluster: |
| 53 | + insecure-skip-tls-verify: true |
| 54 | + server: https://api.my-cluster.dev.rhcloud.com:6443 |
| 55 | + name: api-my-cluster-dev-rhcloud-com:6443 |
| 56 | + contexts: |
| 57 | + - context: |
| 58 | + cluster: api-my-cluster-dev-rhcloud-com:6443 |
| 59 | + namespace: default |
| 60 | + user: kube:admin/api-my-cluster-dev-rhcloud-com:6443 |
| 61 | + name: default/api-my-cluster-dev-rhcloud-com:6443/kube:admin |
| 62 | + current-context: default/api-my-cluster-dev-rhcloud-com:6443/kube:admin |
| 63 | + kind: Config |
| 64 | + preferences: {} |
| 65 | + users: |
| 66 | + - name: kube:admin/api-my-cluster-dev-rhcloud-com:6443 |
| 67 | + user: |
| 68 | + token: sha256~ABCdEF(...) # fake secret, obviously |
| 69 | +``` |
| 70 | +
|
| 71 | +When a GitOpsDeploymentManagedEnvironment/Secret are created: |
| 72 | +
|
| 73 | +1. The GitOps Service creates a new **ServiceAccount/Secret** on the target cluster |
| 74 | + * Using the credentials the user provided, but those are only used initially. |
| 75 | +2. It then creates a '\* \* \*' **ClusterRole/ClusterRoleBinding** for that ServiceAccount. |
| 76 | +3. Next, the Service **stores the credentials** for that new service account: |
| 77 | + * the API URL, and the service account token. |
| 78 | + * Together, these will be used by and allow Argo CD to login to the cluster via this ServiceAccount. |
| 79 | +
|
| 80 | +The GitOps Service thus ONLY uses the user-provided credentials (those provided in the Secret) for the initialization of the ManagedEnvironment/Secret. From that point on, it uses a ServiceAccount on the target cluster created by the GitOps Service. |
| 81 | +
|
| 82 | +This current behaviour makes sense when the user is using a Kubernetes *User* account to create the cluster credentials (which will be based on a user K8s session that will expire), but makes less sense when the user is giving us the credentials for an existing service account (which should not expire). |
| 83 | +
|
| 84 | +I believe the first use case (credentials based on a Kubernetes User account) is why the ‘argocd cluster add’ command has the behaviour that it does, and we followed this behaviour here. |
| 85 | +
|
| 86 | +## Problems with the current behaviour |
| 87 | +
|
| 88 | +The current behaviour doesn't work well when the user already has a ServiceAccount setup on a cluster, which they wish Argo CD to use. |
| 89 | +
|
| 90 | +This is especially true when the ServiceAccount they wish Argo CD to use is not cluster-scoped: for example, only being able to deploy to a fixed number of namespaces. |
| 91 | +
|
| 92 | +* The ServiceAccount, for example, might use Roles/RoleBindings on a small set of Namespaces, rather than a single \* \* \* ClusterRole/Binding |
| 93 | +
|
| 94 | +It's this exact case \-- wanting to use an existing ServiceAccount \-- that is driving the change described here. |
| 95 | +
|
| 96 | +One driver of this requirement is StoneSoup: Since we've dropped support for KCP, StoneSoup has been exploring alternative mechanisms for supporting multiple users on a single cluster. |
| 97 | +
|
| 98 | +* This includes the ability to allow untrusted users to deploy to multiple namespaces on a shared cluster. |
| 99 | +* For example: |
| 100 | + * 'jgwest-appA', 'jgwest-appB'. Or, 'jgwest-appA-staging', 'jgwest-appB-prod'. |
| 101 | + * And on the same cluster, there might also be 'wtam-appA', 'wtam-appB', and so on. |
| 102 | +
|
| 103 | +The current proposed mechanism to achieve this is SpaceRequests and new AppStudio Environment APIs, which are outside the scope of this document. |
| 104 | +
|
| 105 | +In any case, what the GitOps Service will get out of this process is an existing credentials/API URL for ServiceAccount. The ServiceAccount will NOT have \* \* \* ClusterRole/RoleBinding, and thus the existing behaviour will fail: the logic will attempt to create a CR/CRB on the cluster, and not be able to, due to lack of permissions. |
| 106 | +
|
| 107 | +## New proposed behaviour |
| 108 | +
|
| 109 | +**TL;DR**: We use the cluster credential exactly as provided by the user. |
| 110 | +
|
| 111 | +When a GitOpsDeploymentManagedEnvironment/Secret is created: |
| 112 | +
|
| 113 | +1) We test that the credentials work: that we are able to connect to the target cluster using that API URL/token |
| 114 | +2) We set up Argo CD to use those: the API URL, and the service account token. |
| 115 | +3) (Unlike with the original behaviour, we do not create a new ServiceAccount on the target cluster) |
| 116 | +
|
| 117 | +Together, these will allow Argo CD to login to the cluster via the user-provided credentials. |
| 118 | +
|
| 119 | +## Options |
| 120 | +
|
| 121 | +So the main open question is whether to support both options, or support only the new behaviour. |
| 122 | +
|
| 123 | +**A) Support both old (argocd-cli-like) and new (serviceaccount-centric) behaviour** |
| 124 | +
|
| 125 | +* Requires an API change: user must indicate which behaviour they want. |
| 126 | +* This option is my preference, as it allows us support both use cases: a user can specified whether they want us to create a new ServiceAccount based on the provided credentials, or just use what they have provided directly |
| 127 | +
|
| 128 | +**B) Support only new behaviour** |
| 129 | +
|
| 130 | +* Does not require an API change: we just switch the code/tests to use the new behaviour. |
| 131 | +
|
| 132 | +**C) ~~Support only the old behaviour~~** |
| 133 | +
|
| 134 | +* I think this is untenable, due to the problem identified above. |
| 135 | +
|
| 136 | +### And, if we go with option A) \- what API do we want |
| 137 | +
|
| 138 | +If we go with Option A), we will need to make an API change to allow the user (the user is the creator/consumer of the **GitOpsDeploymentManagedEnvironnment** CR) to specify which behaviour they want. |
| 139 | +
|
| 140 | +```yaml |
| 141 | +kind: GitOpsDeploymentManagedEnvironment |
| 142 | +spec: |
| 143 | + apiURL: (api url containing credentials to use for deployment) |
| 144 | + credentialsSecret: (secret containing credentials to use for deployment; these will be provided as is to Argo CD) |
| 145 | + allowInsecureSkipTLSVerify: (...) |
| 146 | + |
| 147 | + createNewServiceAccount: true/false # new field: false is default. if true, will follow option A behaviour. If false, option B. |
| 148 | +``` |
| 149 | +
|
| 150 | +
|
| 151 | + |
| 152 | +Ideas for new field names: |
| 153 | +
|
| 154 | +* **createNewServiceAccount** (descriptive, but it isn't very declarative) |
| 155 | +* **createNewDeploymentServiceAccount** (more descriptive, getting too long, not very declarative) |
| 156 | +* **serviceManagedServiceAccount** (whether or not the GitOps Service should create and manage a new ServiceAccount on the cluster, description but confusing terminology ) |
| 157 | +* **managedServiceAccount** (managed term is overloaded, and thus not especially descriptive here) |
| 158 | +* **shouldWeCreateAndManageAServiceAccountOnTheClusterInsteadOfUsingTheCredentialsYouGiveUs** 😛 |
| 159 | +* **useExistingSecret** or **useExistingServiceAccount** |
| 160 | +* **importServiceAccount** |
| 161 | +* others? |
| 162 | +
|
| 163 | +**Another option that was proposed in cabal, was a user specifying the particular SA to use:** |
| 164 | +
|
| 165 | +```yaml |
| 166 | +kind: GitOpsDeploymentManagedEnvironment |
| 167 | +spec: |
| 168 | + apiURL: (api url containing credentials to use for deployment) |
| 169 | + credentialsSecret: (secret containing credentials to use for deployment; these will be provided as is to Argo CD) |
| 170 | + allowInsecureSkipTLSVerify: (...) |
| 171 | + |
| 172 | + # option c), this new field: |
| 173 | + remoteClusterServiceAccount: |
| 174 | + name: my-service-account |
| 175 | + namespace: jgw |
| 176 | +``` |
| 177 | +
|
| 178 | +I like the idea of this, but there are a few issues with this option: |
| 179 | +
|
| 180 | +* The cluster credentials provided by the user in credentialSecret might be different from the ones provided in remoteClusterServiceAccount |
| 181 | + * Which is fine, but there’s no reason for them to be different. This is a configuration option without a purpose. |
| 182 | +* If this field is not specified, a serviceaccount will automatically be created. This violates the principle of least surprise. |
| 183 | +
|
0 commit comments