Prevent chart CR creation if managed cm and secret are not ready

[corest]

Towards https://github.com/giantswarm/giantswarm/issues/14604

If app CR has annotation config.giantswarm.io/version - it means cm/secret will be configured by config-controller.
And if those values are not set yet - it means config-controller didn't finish its job yet, so cancelling chart/cm/secret resources reconciliation

Checklist

• Update changelog in CHANGELOG.md.

[kubasobon]

Serves the purpose from my perspective

[rossf7]

@corest Thanks good to add this. My only query is whether we could use the validation resource for this?

This was added recently and executes first. If the CR is not valid it cancels the reconciliation and sets the reason field in the CR status.

Resource is here.
https://github.com/giantswarm/app-operator/blob/master/service/controller/app/resource/validation/create.go#L23-L30

The logic is in the app library. As we will use the same validation rules in app-admission-controller.
https://github.com/giantswarm/app/blob/2ec52e388c1a0ec7b964bf2301f71fd4e6b40407/pkg/validation/app.go#L22

[kopiczko]

Let's merge giantswarm/apiextensions#629 first and use it here.

[kopiczko]

The logic is in the app library. As we will use the same validation rules in app-admission-controller.
https://github.com/giantswarm/app/blob/2ec52e388c1a0ec7b964bf2301f71fd4e6b40407/pkg/validation/app.go#L22

But shouldn't admission controller allow the app without CM/Secret set? Otherwise we won't be able to create an App CR for the config-controller to reconcile. Am I missing sth?

[corest]

But shouldn't admission controller allow the app without CM/Secret set? Otherwise we won't be able to create an App CR for the config-controller to reconcile. Am I missing sth?

good catch. In general it is important to clarify what is validation in this case? Because App CR without cm/secret but with annotation that cm/secret are managed - is still valid CR. It is just that cm/secret are expected to be created later. And if we use e.g. this approach https://github.com/giantswarm/app/pull/52/files only in app-operator - that's gonna work. But if we put it into admission-webhook - so I don't know then - it is still valid CR, but what, we will block its creation?

[rossf7]

But if we put it into admission-webhook - so I don't know then - it is still valid CR, but what, we will block its creation?

@corest @kopiczko I think this will be OK. The app CR will be created but not processed and the reason will be added to the status.

I've been a bit stealthy with app-admission-controller. Full plan is here https://github.com/giantswarm/giantswarm/issues/11656

It's mainly to help with tenant app CRs. These are more complex than CP apps CRs with the kubeconfig secret etc. We also regularly get problems like user values configmaps that don't exist.

We want to roll it out via platform releases. So the proposal is it will only apply when app-operator.giantswarm.io/version label >= 3.0.0. Meaning unique app CRs are just accepted with no defaulting or validation.

We can add that later if needed but I also want to try and avoid deadlock situations or having to delete the webhooks manually which would be a pain.

[kopiczko]

Still it doesn't sound correct to me. As @corest said the App CR without CM/Secret set is valid. But if you prefer it that way I won't argue.

[rossf7]

Still it doesn't sound correct to me. As @corest said the App CR without CM/Secret set is valid.

The app CR is valid but we don't want to process it yet. Plus the reason it wasn't processed gets added to the app CR status which is important for trouble shooting.

It also means if we add or change the other resources this check will persist.

[kopiczko]

Sorry, I'm not deep into the details. app-admission-controller + validation sounds like it could prevent App CR from being created. If you say it will be ok after I pointed out my concerns - I'm fine. Let's proceed.

[corest]

Done here #598