Skip to content
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

Implement NKG-specific field validation #363

Closed
3 tasks done
Tracked by #308
pleshakov opened this issue Jan 11, 2023 · 0 comments
Closed
3 tasks done
Tracked by #308

Implement NKG-specific field validation #363

pleshakov opened this issue Jan 11, 2023 · 0 comments
Assignees
Labels
area/gateway/core Relates to all Core features of Gateway area/gatewayclass/core Relates to all Core features of GatewayClass area/httproute/core Relates to all Core features of HTTPRoute enhancement New feature or request
Milestone

Comments

@pleshakov
Copy link
Contributor

pleshakov commented Jan 11, 2023

Parent issue -- #308

Background

NKG must prevent generating NGINX configuration with invalid or malicious values. See more context
in https://github.com/nginxinc/nginx-kubernetes-gateway/blob/6531ca1c51f1d552dae24c3b26939d2f29af8797/design/resource-validation.md

User Stories

Cluster admin and application developer = user

  1. As a user if I create/update an invalid resource, I want NKG to ignore the whole or only the affected part of the
    resource (according to Gateway API spec) and report the validation error in its status.

Requirements

Functional Requirements

We assume an invalid resource passed both the CRD and Webhook validations.

  1. If a user creates a resource and NKG finds it invalid, NKG ignores the whole or only the affected part of the
    resource (according to Gateway API spec) and reports the error in its status.
  2. If a user updates a resource and NKG finds it invalid, NKG ignores the whole or only the affected part of the
    resource (according to Gateway API spec), removing any previous NGINX config for those, and reports the error in its
    status.

Resources and Fields to Validate

GatewayClass

spec

  • controllerName - must be equal to the NKG controller name
  • parametersRef - NKG does not support it.

If controllerName doesn't match the controller name of NKG, NGK will:

  • not update the resource status (because the resource belongs to a different controller)
  • not process other resources (Gateway, HTTPRoutes, ...), because they belong to a GatewayClass of a different controller

Note that the Webhook, if it is running properly, makes the field controllerName immutable.

If parameterRef is set, NGK will:

  • update the resource status: condition Accepted, status False, reason InvalidParameters
  • process other resources (Gateway, HTTPRoutes, ...), report errors related to GatewayClass.

If it doesn't exist:

  • process other resources (Gateway, HTTPRoutes, ...), report errors related to non-existing GatewayClass.

Gateway

spec

  • listeners
    • hostname - gets into NGINX config.
    • port - NKG only supports 80 for HTTP and 443 for HTTPs.
    • protocol - NKG only supports HTTP and HTTPs.
    • tls - required for HTTPS
      • mode - only Terminate
      • certificateRefs - only one ref, only Secret kind, only same namespace *1
      • options - NKG does not support any options
    • allowedRoutes - NKG does not support.
  • addresses - NGK does not support.

*1 - validation of the TLS Secret content is covered in #359

If a listener is invalid, NKG will report in its status condition Accepted, status False and appropriate reason
from the Gateway API or an NGK-specific reason.

If addresses is set, NKG will report in the status of the listeners of the Gateway its status condition Accepted, status False, reason
UnsupportedAddress.

HTTPRoute

spec

  • parentRefs
    • port - not supported
  • hostnames - gets into NGINX config. Wildcard hostnames are not supported.
  • rules
    • matches
      • path
        • type - NKG does not support PathPrefix, RegularExpression
        • value - gets into NGINX config *1
      • headers
        • type - NGK does not support RegularExpression
        • name - gets into NGINX config.
        • value - gets into NGINX config.
      • queryParams
        • type - NGK does not support RegularExpression
        • name - gets into NGINX config.
        • value - gets into NGINX config.
      • method - gets into NGINX config. NGINX does not support CONNECT, TRACE methods (it
        will return 405 Not Allowed to clients). But no config reload failure though.
    • filters
      • type - NKG only supports RequestRedirect
      • requestRedirect
        • scheme - NKG must report unsupported values
        • hostname - gets into NGINX config.
        • path - not supported.
        • statusCode - NKG must report unsupported values.
    • backendRefs
      • reference - NGK only supports Service in the same namespace
      • filters - NKG does not support.

*1 - for path validation, ensure whitespace is trimmed to prevent duplicate locations. See this
comment #356 (comment) (no longer relevant -- the webhook validation doesn't allow whitespace)

If field (of fields) is invalid, NKG will report it its status condition Accepted, status False with an appropriate reason. Except for field(s) of a backend ref. In that case, NKG will report condition ResovledRefs status False.

Edge Cases

If for some reason an invalid value is still propagates to NGINX config, NGINX will fail to reload. This means NGK validation or config generation has a bug. Such failures must be reported to the cluster admin. This will be covered in #292

Aha! Link: https://nginx.aha.io/features/NKG-27

@pleshakov pleshakov added enhancement New feature or request and removed proposal labels Jan 19, 2023
@pleshakov pleshakov self-assigned this Jan 19, 2023
@kate-osborn kate-osborn added area/httproute/core Relates to all Core features of HTTPRoute area/gateway/core Relates to all Core features of Gateway area/gatewayclass/core Relates to all Core features of GatewayClass labels Mar 21, 2023
@kate-osborn kate-osborn added this to the v0.3.0 milestone Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gateway/core Relates to all Core features of Gateway area/gatewayclass/core Relates to all Core features of GatewayClass area/httproute/core Relates to all Core features of HTTPRoute enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

2 participants