🐛 Fix watchNamespace configuration validation #2468
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| installNamespace: "install-ns", | ||
| expectedErrSubstrings: []string{ | ||
| "field \"watchNamespace\"", | ||
| "must match pattern \"^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$\"", |
There was a problem hiding this comment.
IHMO: We should use machinary lib instead of regex here
That has a func to validate it
There was a problem hiding this comment.
If we're using a jsonschema to validate input, shouldn't we rely as much as possible on what it offers?
There was a problem hiding this comment.
I took the regex from that lib though
There was a problem hiding this comment.
keep in mind the mental model is: bundle provides jsonschema, olm validates user input against jsonschema. So, we should use custom formats sparingly.
There was a problem hiding this comment.
I think it is fine 👍 fair enough
There was a problem hiding this comment.
Pull request overview
This PR adds validation for the watchNamespace configuration parameter to ensure it conforms to Kubernetes namespace naming rules. The changes address a gap where invalid namespace names were not being rejected during bundle configuration validation, particularly for bundles with single namespace install modes.
Changes:
- Added minLength, maxLength, and pattern validation to the watchNamespace schema definition
- Enhanced error formatting to handle pattern, minLength, and maxLength validation errors with type-based matching instead of string matching
- Added test cases to verify namespace format validation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| internal/operator-controller/rukpak/bundle/registryv1.go | Added namespace validation constraints (pattern, min/maxLength) to watchNamespace schema; converted exported constant to unexported |
| internal/operator-controller/config/config.go | Refactored error formatting to use type-based error kind matching; removed invalid value display from custom format validator error messages; added handlers for pattern, minLength, and maxLength errors |
| internal/operator-controller/config/error_formatting_test.go | Added test cases for namespace pattern validation and updated expected error messages to match new format |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
perdasilva
force-pushed
the
fix-namespace-format-validation
branch
from
a8bf0f3 to
650719c
Compare
perdasilva
force-pushed
the
fix-namespace-format-validation
branch
from
650719c to
6c0fd75
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
perdasilva
force-pushed
the
fix-namespace-format-validation
branch
from
6c0fd75 to
a284284
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2468 +/- ##
==========================================
- Coverage 69.52% 69.48% -0.05%
==========================================
Files 102 102
Lines 8231 8249 +18
==========================================
+ Hits 5723 5732 +9
- Misses 2056 2063 +7
- Partials 452 454 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
perdasilva
force-pushed
the
fix-namespace-format-validation
branch
from
a284284 to
228d5ab
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
perdasilva
force-pushed
the
fix-namespace-format-validation
branch
from
228d5ab to
006f2fb
Compare
perdasilva
force-pushed
the
fix-namespace-format-validation
branch
from
006f2fb to
f790949
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
perdasilva
force-pushed
the
fix-namespace-format-validation
branch
from
f790949 to
6e173e7
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
openshift-merge-bot
bot
merged commit 4d4f894
into
operator-framework:main
2 participants
Description
The watchNamespace configuration parameter was not being validated to check if it is a valid kubernetes namespace. In bundles that support single namespace install modes this can lead to bad namespaces not getting rejected during bundle configuration validation.
Changes:
Note: The jsonschema library does not seem to collect format and other errors. To keep things simple for now, I've avoided adding namespace format checks to the custom single- ownnamespace formats.
Reviewer Checklist