-
Notifications
You must be signed in to change notification settings - Fork 229
Add comprehensive documentation for resource extension points #5004
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
base: main
Are you sure you want to change the base?
Conversation
- Created extension-points folder with overview - Documented ARMResourceModifier - Documented Claimer - Documented Deleter - Documented ErrorClassifier - Documented Importer Co-authored-by: theunrepentantgeek <[email protected]>
- Documented KubernetesSecretExporter - Documented PostReconciliationChecker - Documented PreReconciliationChecker - Documented SuccessfulCreationHandler Co-authored-by: theunrepentantgeek <[email protected]>
Updated interface documentation to provide more context about: - When each extension should be used - What problems each extension solves - Key parameters and return values Enhanced comments for: - ARMResourceModifier - Claimer - Deleter - ErrorClassifier - Importer - KubernetesSecretExporter - PostReconciliationChecker - PreReconciliationChecker - SuccessfulCreationHandler Co-authored-by: theunrepentantgeek <[email protected]>
Updated the contributing index page to link to the new extension points documentation. Co-authored-by: theunrepentantgeek <[email protected]>
|
@copilot Please add documentation for |
Added comprehensive documentation for PreReconciliationOwnerChecker, a specialized variant of PreReconciliationChecker that validates only the owner's state before reconciliation without accessing the resource itself. Key aspects: - Detailed explanation of when to use vs PreReconciliationChecker - Real-world example from Kusto Database (cluster state blocking) - Comparison table showing differences from PreReconciliationChecker - Common patterns for blocking based on owner state - Enhanced code comments in the interface definition This extension is critical for resources where the owner's state can completely block access to child resources (e.g., Kusto databases inaccessible when cluster is powered off). Co-authored-by: theunrepentantgeek <[email protected]>
Added documentation for This extension is a specialized variant of The documentation includes:
|
theunrepentantgeek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good, but there are a few things to tweak, @copilot. Please make the requested changes.
- Fixed comment grammar in importer.go (added "an") - Updated _index.md description to focus on compensating for resource variation - Changed "type assertion" to "type-check at the appropriate time" - Removed Claimer from extension points documentation and deleted claimer.md Claimer is not a contributor-facing extension point but an internal interface. Co-authored-by: theunrepentantgeek <[email protected]>
All requested changes completed in commit df58f62:
|
|
|
||
| The following extension points are available for customizing resource behavior: | ||
|
|
||
| | Extension Point | Purpose | When Invoked | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor, style: maybe format this table to be more human readable in the raw markdown?
|
|
||
| 1. **Keep extensions minimal**: Only add logic that cannot be handled by the generator | ||
| 2. **Document thoroughly**: Explain why the extension is needed | ||
| 3. **Type assert hub versions**: Include hub type assertions to catch breaking changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May want an example?
| --- | ||
| title: Extension Points | ||
| linktitle: Extension Points | ||
| weight: 100 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought lower weight is "higher" on the page. I guess this is the index so this weight is about where this lands compared to othher contributing docs?
So this is at the end?
Just confirming this is correct as it seemed a little surprising to me
|
|
||
| The interface is invoked during PUT and PATCH operations to ARM, giving the resource an opportunity to make last-minute adjustments to the ARM payload based on runtime conditions, Azure state, or complex business logic that cannot be expressed in the generated code. | ||
|
|
||
| ## Interface Definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if we should just link to the code here? Or directly embed the code via link (does Hugo support that? probably not).
Calling the exact structure + what the parameters mean out here just seems like writing an interface/function comment and I'd sorta rather have a single place where we document it well (ideally in the code) and then reference that place from the docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd still keep the "when to use", motivation, examples, etc here.
|
|
||
| The Key Vault resource uses `ARMResourceModifier` to handle different creation modes based on whether a soft-deleted vault exists: | ||
|
|
||
| ```go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another place where we should maybe link to the example code rather than duplicate it here?
| 5. **Test conditional paths**: Cover all branching logic | ||
| 6. **Test requeue behavior**: Verify multi-step deletions requeue correctly | ||
|
|
||
| Example test structure: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we need to see an example test structure here? Seems a bit overkill for the docs.
| } | ||
| ``` | ||
|
|
||
| ### Pattern 5: Multiple Error Conditions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't really a separate pattern this is just pattern 1 again, isn't it?
| 4. **Test error message enhancement**: Verify improved user messages | ||
| 5. **Test classification changes**: Assert correct classification results | ||
|
|
||
| Example test structure: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment, IMO overkill
| Example test structure: | ||
|
|
||
| ```go | ||
| func TestFlexibleServersConfigurationExtension_Import(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
| } | ||
| ``` | ||
|
|
||
| Example test implementation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again seems overkill -- link to an existing test at most?
ASO v2 provides 9 extension interfaces for customizing resource behavior, but lacked documentation causing contributor friction. This adds ~3,700 lines of technical documentation covering all extension points.
Changes
New Documentation Structure
docs/hugo/content/contributing/extension-points/folder with overview and 9 detailed guidesExtension Points Documented
asoctl importNote: Claimer was initially included but removed as it is an internal interface, not a contributor-facing extension point.
Code Enhancement
Enhanced interface comments in
v2/pkg/genruntime/extensions/*.goto include usage guidance and problem statements inline.Example
Each documentation page includes 5-6 implementation patterns with real codebase examples (KeyVault soft-delete, PrivateEndpoint approval, MySQL config filtering, Kusto cluster/database state blocking).
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.