-
Notifications
You must be signed in to change notification settings - Fork 14
Fix nil pointer validation issues in Power Platform Terraform provider #830
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
Conversation
…els, authorization resource, and environment settings Co-authored-by: mawasile <50197777+mawasile@users.noreply.github.com>
Co-authored-by: mawasile <50197777+mawasile@users.noreply.github.com>
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 strengthens nil pointer validation across the Terraform provider to prevent runtime panics by introducing a reusable helper, adding missing checks, and improving error handling.
- Replaced repetitive null/unknown checks with a new
helpers.IsKnown
function - Added missing header existence and parse error handling in the application client
- Enforced required field validations in user resource CRUD methods
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
internal/services/application/api_application.go | Added missing check for operation location header and error handling |
internal/helpers/config.go | Introduced helpers.IsKnown to encapsulate null/unknown checks |
internal/services/environment_settings/models.go | Replaced verbose null/unknown patterns with helpers.IsKnown |
internal/services/environment/models.go | Simplified environment model validations using helpers.IsKnown |
internal/services/authorization/resource_user.go | Added nil/unknown checks for planning and state fields in CRUD operations |
.changes/unreleased/fixed-20250605-070828.yaml | Documented the fix for Issue #815 |
Comments suppressed due to low confidence (2)
internal/services/application/api_application.go:281
- Correct the typo in the debug message: change "Opeartion Location Header" to "Operation Location Header".
tflog.Debug(ctx, "Opeartion Location Header: "+operationLocationHeader)
internal/helpers/config.go:89
- Add unit tests for the new
IsKnown
helper to verify its behavior with null, unknown, and defined attribute values.
func IsKnown(value attr.Value) bool {
…orization resource Co-authored-by: mawasile <50197777+mawasile@users.noreply.github.com>
This PR addresses three critical nil pointer validation issues that could lead to runtime panics in the Power Platform Terraform provider.
Issues Fixed
Issue 1: Missing nil check for operation location header
File:
internal/services/application/api_application.go
The
InstallApplicationInEnvironment
method was not validating that the operation location header was present before using it, which could cause downstream HTTP requests with empty URLs or crashes.Before:
After:
Issue 2: Repetitive null/unknown validation patterns
Files:
internal/services/environment/models.go
,internal/services/environment_settings/models.go
Created a reusable
helpers.IsKnown()
function to replace verbose null/unknown checks throughout the codebase.Before:
After:
Issue 3: Missing nil/zero-value validation for critical fields
File:
internal/services/authorization/resource_user.go
Added comprehensive validation for plan and state objects in all CRUD operations to prevent nil pointer dereferences.
Before:
After:
Testing
Impact
These changes improve the robustness and reliability of the provider by:
helpers.IsKnown()
utility functionFixes #815.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.