-
Notifications
You must be signed in to change notification settings - Fork 85
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
Blocking project wide ssh keys while creating machine class #506
Blocking project wide ssh keys while creating machine class #506
Conversation
@rishabh-11 Labels area/control, area/plane do not exist. |
@@ -179,7 +179,14 @@ func (w *workerDelegate) generateMachineConfig(_ context.Context) error { | |||
"description": fmt.Sprintf("Machine of Shoot %s created by machine-controller-manager.", w.worker.Name), | |||
"disks": disks, | |||
"labels": gceInstanceLabels, | |||
"machineType": pool.MachineType, | |||
// TODO: make this configurable for the user |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a concrete need already known to let users pass additional metadata for the machines?
If yes, we might wanna create an issue for it as it is better trackable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the current mcm-provider-gcp
code, we have a feature to add metadata
field on a VM instance at the time of its creation(See this). But at the moment we cannot specify metadata field in the providerConfig
section of the worker pool in the shoot
yaml as the current extension-provider-gcp
cannot decode it.(See this and this). So even though mcm provides the feature of specifying metadata on a VM instance, it will not be used as the machine class will never have this field present. This is the reason why I added this comment for now.
I am not opening an issue right now because we are unsure if we want to keep this field in the machine class. The reason is that any change in the providerConfig
part of the worker
section in the shoot
yaml will generate a new machine class name (See this and this) and therefore a new machine class which will cause a rolling update of the machines. We don't want this just because some metadata
field was changed. We already have an issue on mcm (See this) which requires something similar and should be sufficient to keep track of this as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
How to categorize this PR?
/area control-plane
/kind enhancement
/platform gcp
What this PR does / why we need it:
In GCP, for each project, there is a
project-wide ssh key
which can be used to access all the VM instances in that project. Each Instance has an option to disable this by markingBlock project-wide SSH keys
asOn
from the GCP console. This PR adds ametadata
field in the machine class spec during machine class creation, which will then be used bymcm-provider-gcp
during VM instance creation to mark theBlock project wide SSH keys
field asOn
. The corresponding code inmcm-provider-gcp
can be checked out here. Themetadata
field will be present in theproviderSpec
ofmachineclass
and will look like this:Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
The testing of this was done using a shooted seed local setup of gardener, and it worked as expected. The created machine had
Block project wide SSH keys
asOn
in the GCP console.Release note: