elastisys / compliantkubernetes-kubespray

Apache License 2.0
28 stars 7 forks source link

Force sc wc cluster names #330

Closed anders-elastisys closed 11 months ago

anders-elastisys commented 11 months ago

What kind of PR is this?

Required: Mark one of the following that is applicable:

Optional: Mark one or more of the following that are applicable:

[!important] Critical security fixes should be marked with kind/security Breaking changes should be marked kind/admin-change or kind/dev-change depending on type

Platform Administrator notice

ck8s-kubespray commands no longer support using other prefixes besides sc or wc.

What does this PR do / why do we need this PR?

The compliantkubernetes-apps repo forces cluster config names to be wc and sc while the compliantkubernetes-kubespray repo has previously supported custom prefixes for cluster config. This PR helps standardize the repositories by enforcing the wc and sc cluster naming convention.

Additional information to reviewers

Screenshots

Checklist

anders-elastisys commented 11 months ago

I think it would make sense to also verify that the prefix is correct here as well:

https://github.com/elastisys/compliantkubernetes-kubespray/blob/37f7386c39db2a9c806f935687ea0c42d824fd25/bin/common.bash#L8

Maybe there are other places in the code where it would also make sense.

@viktor-f I added a check in common.bash, as I see it most of the other bin scripts sources this script, hence they will all run this check. However, I noticed that common.bash runs for all scripts (since it is sourced here) and it the prefix to set different config variables. This does not really make sense for the upgrade command in its current state, as it runs for both clusters. Should this be addressed in this PR (or perhaps in this one), or should it perhaps be a separate issue? It does not really break anything as it is, its more that it doesn't entirely make sense.

viktor-f commented 11 months ago

I think it would make sense to also verify that the prefix is correct here as well: https://github.com/elastisys/compliantkubernetes-kubespray/blob/37f7386c39db2a9c806f935687ea0c42d824fd25/bin/common.bash#L8 Maybe there are other places in the code where it would also make sense.

@viktor-f I added a check in common.bash, as I see it most of the other bin scripts sources this script, hence they will all run this check. However, I noticed that common.bash runs for all scripts (since it is sourced here) and it the prefix to set different config variables. This does not really make sense for the upgrade command in its current state, as it runs for both clusters. Should this be addressed in this PR (or perhaps in this one), or should it perhaps be a separate issue? It does not really break anything as it is, its more that it doesn't entirely make sense.

I think it makes sense to try to fix it in this PR (but maybe after the other PR is merged). Not sure exactly how to fix it though, it would be nice if we just allow "both" for the upgrade command and then "wc/sc" for the rest.