-
Notifications
You must be signed in to change notification settings - Fork 96
Support Linode Integrations with ACLP Alerts #879
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: proj/aclp-linode-alerts
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR adds support for ACLP (Access Control List Policy) alerts to Linode instance operations, enabling system and user alert configurations during instance creation, updates, and cloning.
Changes:
- Extended
InstanceAlertstruct to includeSystemAlertsandUserAlertsfields - Added
InstanceACLPAlertsOptionstype for configuring alerts during instance creation and cloning - Updated instance operation options (
InstanceCreateOptions,InstanceUpdateOptions,InstanceCloneOptions) to support alert configuration - Added comprehensive test coverage for alert functionality across Get, Create, Update, and Clone operations
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| instances.go | Added ACLP alert fields to InstanceAlert struct and new InstanceACLPAlertsOptions type for alert configuration in create/clone operations |
| test/unit/instance_test.go | Added test assertions to verify SystemAlerts and UserAlerts are properly handled in Get, Create, Update, and Clone operations |
| test/unit/fixtures/instance_get.json | Added alerts field with system_alerts and user_alerts arrays to mock response data |
| test/unit/fixtures/instance_create.json | Added alerts field with system_alerts and user_alerts arrays to mock response data |
| test/unit/fixtures/instance_update.json | Added alerts field with system_alerts and user_alerts arrays to mock response data |
| test/unit/fixtures/instance_clone.json | Added alerts field with system_alerts and user_alerts arrays to mock response data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Alerts *InstanceACLPAlertsOptions `json:"alerts,omitempty"` | ||
| } | ||
|
|
||
| // InstanceACLPAlertsOptions represents ACLP alerts options for instance creation and cloning. |
Copilot
AI
Jan 23, 2026
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.
The InstanceCreateOptions.Alerts field uses InstanceACLPAlertsOptions, but InstanceUpdateOptions.Alerts (line 226 in the test file) uses InstanceAlert. This inconsistency creates confusion about which type should be used for updates versus creation. Consider using the same type for both operations or clearly documenting why different types are needed.
| Alerts *InstanceACLPAlertsOptions `json:"alerts,omitempty"` | |
| } | |
| // InstanceACLPAlertsOptions represents ACLP alerts options for instance creation and cloning. | |
| // Note: This field intentionally uses InstanceACLPAlertsOptions, which differs | |
| // from the alerts type used in InstanceUpdateOptions (InstanceAlert). The API | |
| // for configuring alerts at creation/cloning time is distinct from the API | |
| // used when updating an existing Instance's alerts. | |
| Alerts *InstanceACLPAlertsOptions `json:"alerts,omitempty"` | |
| } | |
| // InstanceACLPAlertsOptions represents ACLP alerts options for instance creation | |
| // and cloning. This type is distinct from the alerts type used on | |
| // InstanceUpdateOptions (InstanceAlert) to reflect differences in the API for | |
| // initial (create/clone) configuration versus subsequent updates. |
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 API inconsistency in creating and updating request.
📝 Description
Add ACLP alerts support to linode instance operations.
✔️ How to Test