search

Azure / azure-service-operator

Azure Service Operator allows you to create Azure resources using kubectl
https://azure.github.io/azure-service-operator/
MIT License
716 stars 194 forks source link

asoctl export template --crd-pattern validation #3969

Open matthchr opened 4 months ago

matthchr commented 4 months ago

Describe the current behavior The --crd-pattern parameter is not validated by asoctl at all.

Describe the improvement It would be nice to validate the pattern, but there are some gotchas...

Additional context I got partway through doing this but realized it won't work all that well because asoctl isn't necessarily built with the same version as the target version.

For example let's say that the user is attempting to install the experimental release to test a new CRD they've just added, if they try with an older asoctl, we would reject them because their group doesn't exist. They'd need to download the experimental asoctl and use that.

The problem works the opposite way too, if the user is using a newer asoctl but installing an older release, we'd allow crdpatterns that won't actually work for the operator.

We could maybe solve this problem by doing our best in asoctl and then having a --novalidate flag or something that bypassed validation for crdpattern if the user really knows what they're doing... but really the crdpattern validation seems somewhat at odds with letting the user select a version that may not match the asoctl version.

matthchr commented 4 months ago

We could validate against the asoctl version and emit an error, but have a --no-validate flag that turned all of those errors to warnings.

The error could say something like: "Couldn't find group "compoot", it may have been added in a newer ASO version. If you're sure this group exists, pass --no-validate, otherwise correct the group".

It gets more difficult when dealing with newer asoctl's installing older ASO's though, and the Helm chart doesn't have any of this upfront validation, so it might be better to just have the same experience for the asoctl export template command too.

theunrepentantgeek commented 4 months ago

What if we validated by inspecting the YAML file itself, using it as the source of truthh, looking for at least one match for metadata.name?

E.g. the pattern compute/* would result in us looking for name: .*.compute.azure.com and pass as soon as this was found:

metadata:
  ... elided ...
  name: diskencryptionsets.compute.azure.com

But the pattern compoot/* would result in us looking for name: .*.compoot.azure.com, which wouldn't be found and would result in an error.

matthchr commented 4 months ago

The YAML doesn't actually contain the CRDs though, they're embedded in the operator pod. There is a YAML that has all of the CRDs but we'd have to download it and parse it as well, which isn't impossible but feels awkward in a bunch of instances (local files, etc)

theunrepentantgeek commented 4 months ago

We could do the validation using the versions of CRDs that are available in the current build of asoctl but only generate warnings, not errors, so we can still provide guidance without being blocking.

E.g.

WRN This version of asoctl (v2.7.0) does not recognize the following CRD patterns:
WRN compoot/* - did you mean compute/*?
WRN orbital/*
WRN If you are configuring a newer release of ASO, your template may still work. 
INF Your configured template has been exported.