-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
[bug]: Nullable causes model binding error in webhooks #753
Comments
I meant to add to this that I used that Microsoft doc to add this line: builder.Services
.AddControllers(o => o.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes = true); That seems to have fixed the issue. However, I think there is a chance that anyone using webhooks could run into this, and it was a pretty obscure problem to diagnose. I realize this should probably be fixed in the k8s lib, but it would be nice if the operator had a workaround until kubernetes-client/csharp#606 is fixed. |
Hey @ian-buse Is this still an issue? as far as I saw, they closed the issue over at the k8s client. |
They closed it because they don't have enough contributors to fix it. :( It's still a problem. |
Hey @ian-buse Just encountered this specific issue while updating the dependencies in #764. When I set all these fields as "required" (via If I understand you correctly, the request that is sent does not match the nullable enabled model? Do you have any idea how we might fix this issue? I don't know if I'm able to allow a lax model binding per request. If |
The only thing I've found that fixed the issue is using var builder = WebApplication.CreateBuilder(args);
builder.Services
.AddControllers(o => o.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes = true); Yeah, the k8s library doesn't support nullable types, and in MVC, the reference types (like the UID string) are considered required by default if they are not nullable. So, when MVC tries to bind the UID and finds it missing in the webhook, it fails because the model expects it to be there. I looked into adding something in using KubeOps.Abstractions.Builder;
using KubeOps.Operator.Builder;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;
namespace KubeOps.Operator.Web;
/// <summary>
/// Method extensions for the <see cref="IServiceCollection"/>.
/// </summary>
public static class ServiceCollectionExtensions
{
/// <summary>
/// Add the Kubernetes operator to the dependency injection.
/// </summary>
/// <param name="services"><see cref="IServiceCollection"/>.</param>
/// <param name="operatorConfigure">An optional configure action for adjusting settings in the operator.</param>
/// <param name="mvcConfigure">An optional configure action for the webhook MVCs. Defaults to suppressing required
/// attributes for non-nullable reference types if no configuration is provided.</param>
/// <returns>An <see cref="IOperatorBuilder"/> for further configuration and chaining.</returns>
public static IOperatorBuilder AddKubernetesOperator(
this IServiceCollection services,
Action<OperatorSettings>? operatorConfigure = null,
Action<MvcOptions>? mvcConfigure = null)
{
services
.AddControllers(
mvcConfigure ?? (o => o.SuppressImplicitRequiredAttributeForNonNullableReferenceTypes = true));
var settings = new OperatorSettings();
operatorConfigure?.Invoke(settings);
return new OperatorBuilder(services, settings);
}
} This would require making the Or, maybe some documentation on the issue would suffice, like adding it to the examples and the README. |
Describe the bug
Enabling nullability in my operator project causes webhooks to fail. When nullability is enabled, I see the following
Request.Object.Uid is required
error on the kube-apiserver, which it is receiving from ASP.NET:I0425 03:24:52.491708 1 request.go:1411] body was not decodable (unable to check for Status): Object 'Kind' is missing in '{"type":"https://tools.ietf.org/html/rfc9110#section-15.5.1","title":"One or more validation errors occurred.","status":400,"errors":{"Request.Object.Uid":["The Uid field is required."]},"traceId":"00-1e0d8715878c6606b3c70f8683bd85d3-c57cbcf6dfc4b1a6-00"}' W0425 03:24:52.491764 1 dispatcher.go:225] Failed calling webhook, failing closed mutate.test.company.com.v1: failed calling webhook "mutate.test.company.com.v1": failed to call webhook: the server rejected our request for an unknown reason
And the AdmissionReview Kubernetes is sending in the POST has the following fields for the object (metadata and spec removed for brevity):
And what little MVC logging there was:
trce: Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker[52] Action Filter: Before executing OnActionExecuting on filter Microsoft.AspNetCore.Mvc.Infrastructure.ModelStateInvalidFilter. dbug: Microsoft.AspNetCore.Mvc.Infrastructure.ModelStateInvalidFilter[1] The request has model state errors, returning an error response.
It seems that the model binder is trying to bind the
uid
field for the object, but because 1) nullable is enabled, 2) theUid
is not nullable ink8s.models.V1ObjectMeta
, and 3) the request does not contain theuid
under therequest.object
, it is failing to bind.Some Microsoft documentation might be helpful.
To reproduce
Expected behavior
Model binding should succeed with nullable-enabled projects.
Screenshots
No response
Additional Context
No response
The text was updated successfully, but these errors were encountered: