Closed
Conversation
manavrajvanshi
assigned
manavrajvanshi and
sasikanthmasini and unassigned
manavrajvanshi
sasikanthmasini
added a commit
that referenced
this pull request
Mar 19, 2026
manavrajvanshi
pushed a commit
that referenced
this pull request
Mar 31, 2026
…and LCMConfig data Bug (#208) * Add ConfigMap defaults support for Database CR Allow developers to specify minimal inputs in Database CR with admin-provided defaults via ConfigMap. Values in CR take precedence over ConfigMap defaults. * Update README with ConfigMap defaults documentation * Fix webhook validation to relax checks when ConfigMap defaults are specified This commit adds the critical logic to skip validation for fields that can be provided via ConfigMap defaults (cluster, size, profiles, timeMachine). Key changes: - Webhook detects when defaultsConfigMapRef is set - Passes hasDefaultsConfigMap flag to validation handlers - Validation handlers skip cluster/size/TM checks when ConfigMap ref is present - Allows minimal Database CRs to pass validation successfully Tested: Successfully provisioned databases using minimal CRs with ConfigMap defaults. * Update ConfigMap defaults example with database-specific dbParam profiles - Add postgres.profiles.dbParam.name for PostgreSQL-specific defaults - Add mysql.profiles.dbParam.name for MySQL-specific defaults - Add MySQL-specific example section in ConfigMap - Clarify that dbParam profiles should be database-type-specific * Add auto-snapshot selection for cloning - Updated webhook validation to allow omitting both snapshotId and snapshotName when defaultsConfigMapRef is set - Added automatic selection of latest snapshot when both snapshot fields are empty - Implemented GetLatestSnapshotForTM() in ndb_api to fetch most recent snapshot by timestamp - Updated README with documentation on auto-snapshot feature Testing: Verified with PostgreSQL and MySQL clones, backward compatibility maintained * Decouple auto-snapshot from ConfigMap defaults Changes: - Removed ConfigMap requirement for auto-snapshot feature - Auto-snapshot now works when both snapshotId and snapshotName are omitted - Updated webhook validation: removed hasDefaultsConfigMap check - Updated test: changed from expecting error to expecting success - Updated documentation: clarified auto-snapshot works in any scenario This makes features orthogonal: - ConfigMap defaults = provide default configuration values - Auto-snapshot = automatically select latest snapshot Benefits: - Better UX: no need for ConfigMap just to get auto-snapshot - More flexible: supports common "clone latest" use case - Simpler mental model: omit snapshot = latest Testing: - All 7 webhook validation scenarios pass - Auto-snapshot works without ConfigMap (new behavior) - Backward compatibility maintained - ConfigMap + auto-snapshot still works * Fix nil pointer panic when cloning with LCMConfig in additionalArguments (ERA-57641) - Initialize req.LcmConfig before dereferencing to avoid panic - Fix refreshDetails error condition: use refreshDetailsCount instead of databaseLcmConfigCount - Fix typo in error message: refreshInDay -> refreshInDays * ConfigMap defaults via defaulter webhook (PR #209 approach) * Remove controller_adapters configmap_defaults (moved to api/v1alpha1) * Add unit tests for configmap defaults * unit tests * docs: clarify ConfigMap defaults in README - Document empty ConfigMap data, RBAC (get/list ConfigMaps), and precedence - Split engine size vs profile keys; size is provision-only - Note OOB vs ConfigMap for profile slots; fix typos (Default, Development) Made-with: Cursor * go version and trivy change * pr comments * go changes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.