node-feature-discovery icon indicating copy to clipboard operation
node-feature-discovery copied to clipboard

apis/nfd/validate: loosen validation of feature annotations

Open marquiz opened this issue 1 year ago • 4 comments

Don't require that the annotation value must conform to the (strict) requirements of label values. In the Kubernetes API annotation values do not have other restrictions than that the total size (keys and values) of all annotations combined of an object must not exceed 256kB.

This patch sets a maximum size limit of 1kB for the value of a single feature annotation created by NFD. This limit is rather arbitrary but should be enough for the NFD usage scenarios (until proven wrong).

Also add license header to the test.go file and fix one bug in MatchFeature validation.

marquiz avatar Mar 19 '24 14:03 marquiz

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: marquiz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Mar 19 '24 14:03 k8s-ci-robot

Deploy Preview for kubernetes-sigs-nfd ready!

Name Link
Latest commit 7fbada8b8611c77371c6c6491c63df3aeaa6c106
Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/6617d05f95d7410008dc7670
Deploy Preview https://deploy-preview-1633--kubernetes-sigs-nfd.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Mar 19 '24 14:03 netlify[bot]

/assign @ArangoGutierrez

marquiz avatar Mar 20 '24 09:03 marquiz

Rebased. @ArangoGutierrez PTAL

marquiz avatar Apr 09 '24 10:04 marquiz

/retest

marquiz avatar Apr 09 '24 13:04 marquiz

ping @ArangoGutierrez

marquiz avatar Apr 10 '24 08:04 marquiz

PR updated. @PiotrProkop PTAL

marquiz avatar Apr 11 '24 12:04 marquiz

/lgtm

PiotrProkop avatar Apr 11 '24 14:04 PiotrProkop

LGTM label has been added.

Git tree hash: 5721fdbbbd2bcbabc9fe7f10eed738f321d73e8d

k8s-ci-robot avatar Apr 11 '24 14:04 k8s-ci-robot

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 39.56%. Comparing base (6b80f65) to head (a9946b2). Report is 4 commits behind head on master.

:exclamation: Current head a9946b2 differs from pull request most recent head 7fbada8. Consider uploading reports for the commit 7fbada8 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1633      +/-   ##
==========================================
+ Coverage   38.61%   39.56%   +0.95%     
==========================================
  Files          80       80              
  Lines        6835     6840       +5     
==========================================
+ Hits         2639     2706      +67     
+ Misses       3940     3881      -59     
+ Partials      256      253       -3     
Files Coverage Δ
pkg/apis/nfd/validate/validate.go 100.00% <100.00%> (+44.92%) :arrow_up:

codecov[bot] avatar Apr 11 '24 14:04 codecov[bot]

Thanks @PiotrProkop for the review!

marquiz avatar Apr 11 '24 15:04 marquiz