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

Defining new types for Cloudstack AvailabilityZone development #2412

Merged
merged 27 commits into from
Jun 27, 2022
Merged
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f01e5f0
Defining new types for Cloudstack multiendpoint development
maxdrib Jun 15, 2022
4e9717f
Adding unit test
maxdrib Jun 15, 2022
48a49c6
Adding Name and generating manifests
maxdrib Jun 16, 2022
7a1a24b
Fixing incorrect json label
maxdrib Jun 16, 2022
e0209e1
Deprecating old fields and marking them as optional
maxdrib Jun 16, 2022
d5a4cb3
Removing premature deprecation comment and linting
maxdrib Jun 16, 2022
79d1698
Removing deprecated flag
maxdrib Jun 17, 2022
836e096
Adding unit tests
maxdrib Jun 17, 2022
8732bec
Linting
maxdrib Jun 17, 2022
8dd9e44
More unit testing
maxdrib Jun 17, 2022
5d59551
responding to comments
maxdrib Jun 20, 2022
545d2d1
Updating manifests
maxdrib Jun 20, 2022
d4aa8b6
Generated release manifests
maxdrib Jun 20, 2022
7c6cb84
Adding credentialsRef instead of unique name for az
maxdrib Jun 20, 2022
dd081c4
Generating manifests
maxdrib Jun 20, 2022
2f78096
Merge branch 'main' into multiple-endpoints
maxdrib Jun 21, 2022
b9db281
Updating design to reflect credentialsref
maxdrib Jun 21, 2022
50f7b73
Updating design and readding Name field to AZ struct
maxdrib Jun 22, 2022
83a3828
Updating AZ equality definition to build a map instead of nested for …
maxdrib Jun 22, 2022
aab0350
linting
maxdrib Jun 22, 2022
6201e80
Updating description for credentialsRef
maxdrib Jun 22, 2022
3e56b0a
Adding todo to validator
maxdrib Jun 22, 2022
fd06a1d
Updating design to reflect that secrets should be immutable
maxdrib Jun 23, 2022
75b321c
Adding notes about EKS-A policy on creating but not updating secrets
maxdrib Jun 23, 2022
b353a4e
Flushing out details on credential management in the design
maxdrib Jun 24, 2022
7475853
Responding to Joey's comments
maxdrib Jun 24, 2022
2c76dbb
Update designs/cloudstack-multiple-endpoints.md
maxdrib Jun 27, 2022
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
51 changes: 30 additions & 21 deletions designs/cloudstack-multiple-endpoints.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,7 @@ CAPC currently utilizes them to distribute machines across CloudStack Zones. How
2. Cloudstack domain
3. Cloudstack zone
4. Cloudstack account
5. A reference to the customer-provider credentials

To add more detail to the credential ref, we will continue to accept a base64 encoded ini file as input from the customer when running eks-a. However,
in order to avoid taking a dependency on a meaningless section of a transient file, we will modify EKS-A to also create the secrets to be used by CAPC.
These secrets will be generated from the ini file provided by customer and applied to the cluster. CAPC will then proceed to take ownership of them as
currently done in [CAPV](https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/fae6ef88467e608665e2902e2bb0aaeb4cee67ed/docs/identity_management.md#identity-types).
We will refer to this set of credentials in the CloudstackCluster resource as well so CAPC knows which ones to use.

The secrets will be managed by EKS-A insofar that they can be created but not updated. If users want to update an existing secret, they will have to
do so manually. We can also add a warning if a user is attempting to modify an existing secret. This will provide a safeguard to prevent unintentionally
setting incorrect credentials for a whole collection of clusters.
5. A reference to the customer-provider credentials for interacting with this Cloudstack endpoint

You can find more information about these Cloudstack resources [here](http://docs.cloudstack.apache.org/en/latest/conceptsandterminology/concepts.html#cloudstack-terminology)

Expand All @@ -129,6 +119,9 @@ With the multi-endpoint system for the Cloudstack provider, users reference a Cl
is that all the Cloudstack resources such as image, ComputeOffering, ISOAttachment, etc. must be available in *all* the AvailabilityZones, or all the Cloudstack endpoints,
and these resources must be referenced by name, not unique ID. This would mean that we need to check if there are multiple Cloudstack endpoints, and if so check the zones, networks, domains, accounts, and users.

We will also validate the credentials referenced by each AvailabilityZone, which can either be referenced as existing k8s secrets on the cluster, or from the local ini file. More details
in the Cloudstack Credentials section below

### `CloudstackMachineConfig` Validation

For each CloudstackMachineConfig, we have to make sure that all the prerequisite
Expand All @@ -141,11 +134,10 @@ for availabilityZone in availabilityZones:
validate resource presence with the availabilityZone's configuration of the CloudMonkey executable


### Cloudstack credentials

### Cloudstack Credentials

In a multi-endpoint Cloudstack cluster, each endpoint may have its own credentials. We propose that Cloudstack credentials will be passed in via environment variable in the same way as they are currently,
only as a list corresponding to AvailabilityZones. Currently, these credentials are passed in via environment variable, which contains a base64 encoded .ini file that looks like
only as a list corresponding to some k8s secrets which will be generated. Currently, these credentials are passed in via environment variable, which contains a base64 encoded .ini file that looks like
Copy link
Member

Choose a reason for hiding this comment

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

It's K8s instead of k8s :)


```ini
[Global]
Expand Down Expand Up @@ -175,10 +167,26 @@ api-url = http://172.16.0.3:8080/client/api
...
```

Where the Section names (i.e. Global, AvailabilityZone1, etc.) correspond to the Availability Zone names
Where the Section names (i.e. Global, AvailabilityZone1, etc.) correspond to the credentials needed to access a given Availability Zone.

In order to avoid taking a dependency on a meaningless section of a transient file, we will modify EKS-A to also create the secrets to be used by CAPC.
These secrets will be generated from the ini file provided by customer and applied to the cluster. CAPC will then proceed to take ownership of them as
currently done in [CAPV](https://github.com/kubernetes-sigs/cluster-api-provider-vsphere/blob/fae6ef88467e608665e2902e2bb0aaeb4cee67ed/docs/identity_management.md#identity-types).
We will refer to this set of credentials in the CloudStackCluster resource as well so CAPC knows which ones to use.

So for create, we’ll have:

1. Customer provides ini file containing set of named credentials
2. Customer provides CloudstackDatacenterConfigSpec with list of named AvailabilityZones to distribute the cluster across. Each AvailabilityZone will have a credentialRef which
can either point to an existing secret on the cluster, or a named credential from the ini file
3. EKS-A will check each credentialRef in the AvailabilityZones provided. If it’s already in the cluster as a secret, validate it against the
contents in the ini file. If it’s not already in the cluster as a secret but there’s an entry in the ini file for it, create a new secret. If it’s
not in the cluster as a secret and not in the ini file, throw an error
4. Proceed to generate CAPC template and pass these secretRefs to CAPC who will add an OwnerRef to them

We are also exploring converting the ini file to a yaml input file which contains a list of credentials and their associated endpoints. Either way, this environment variable would
be passed along to CAPC and used by the CAPC controller just like it is currently.
The secrets will be managed by EKS-A insofar that they can be created but not updated. If users want to update an existing secret, they will have to
do so manually both in the k8s secret object, as well as the local ini file prior to running upgrade. We can also add a warning if there is a discrepancy in secret contents between the k8s object and the ini file.
Copy link
Member

Choose a reason for hiding this comment

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

K8s. warning or error out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer error initially and if that's a problem we can relax the constraint.

This will provide a safeguard to prevent unintentionally setting incorrect credentials for a whole collection of clusters.

### Backwards Compatibility

Expand All @@ -188,17 +196,18 @@ In order to support backwards compatibility in the CloudstackDatacenterConfig re

Between these two approaches, we propose to take the first and then deprecate the legacy fields in a subsequent release to simplify the code paths.

However, given that the Cloudstack credentials are persisted in a write-once secret on the cluster, upgrading existing clusters may not be feasible unless CAPC supports overwriting that secret.

## User Experience

Upgrading an existing cluster will require passing the new credentials and templates to CAPC in a way that the mapping from CAPC FailureDomains to EKS-A AvailabilityZones can
be preserved. We plan to align on naming conventions for upgrading existing clusters based on some metadata from the current multi-zone configuration into a multi-endpoint configuration.

## Security

The main change regarding security is the additional credential management. Otherwise, we are doing exactly the same operations - preflight check with cloudmonkey,
create yaml templates and apply them, and then read/write eks-a resources in the eks-a controller. The corresponding change is an extension of an existing mechanism
and there should not be any new surface area for risks than there was previously.

One risk to consider with regards to the new credential management strategy is that if AvailabilityZones are upgraded to use a new credential, there is a possibility of having old rogue Secret objects
which will need to be cleaned up manually.

## Testing

The new code will be covered by unit and e2e tests, and the e2e framework will be extended to support cluster creation across multiple Cloudstack API endpoints.
Expand Down