Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 8, 2025

  • Add initial inactivity_timeout support to Fleet agent policy resource
  • Add version validation for inactivity_timeout (8.7.0+)
  • Add comprehensive test coverage for inactivity_timeout functionality
  • Fix field handling to prevent sending pointer to zero values for unknown attributes
  • Convert inactivity_timeout to user-friendly duration string format
  • Apply code review suggestions for duration conversion and test helper simplification

Changes Made

Duration String Implementation

  • Schema: Added inactivity_timeout as optional, computed String attribute using customtypes.DurationType
  • User Experience: Supports intuitive duration strings like "30s", "2m", "1h" instead of raw seconds
  • API Integration: Automatically converts duration strings to seconds for Fleet API and converts API responses back to duration strings
  • Documentation: Updated to reflect string duration format with examples

Core Features

  • Model: Added field to handle the new attribute in API requests/responses with proper duration conversion
  • API Integration: Updated create and update operations to pass timeout to Fleet API with duration parsing
  • Version Validation: Added minimum version check (8.7.0+) with proper error handling
  • Field Handling: Fixed issue where ValueFloat32Pointer/ValueBoolPointer would send zero values for unknown attributes
  • Testing: Added comprehensive test coverage including explicit timeout configuration and version validation

Code Review Updates

  • Duration Conversion: Improved duration conversion in models.go to use time.Duration with proper truncation instead of string formatting
  • Test Helpers: Simplified test helper functions by removing hardcoded parameters where only single values are used

Field Handling Fix

The previous implementation would always set SupportsAgentless and InactivityTimeout fields in the API request body using ValueBoolPointer() and ValueFloat32Pointer(), which return pointers to zero values (false/0) when the Terraform values are unknown. This could send unintended values to the API.

The fix moves the body declaration above version checks and only sets these fields conditionally when their values are actually known, preventing zero-value pointers from being sent to the API for unknown attributes.

Duration Type Benefits

Users can now specify timeouts using intuitive duration strings:

  • inactivity_timeout = "30s" (30 seconds)
  • inactivity_timeout = "2m" (2 minutes)
  • inactivity_timeout = "1h" (1 hour)

The implementation maintains full compatibility with the Fleet API while providing a much more user-friendly interface that follows Go duration string conventions.


💡 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.

@cla-checker-service
Copy link

cla-checker-service bot commented Sep 8, 2025

❌ Author of the following commits did not sign a Contributor Agreement:
bdb81b4, f6f4a9b, 6cf5c3c, 1be987c, a39c062, 5385514, 4020952, d448c54, cd13249

Please, read and sign the above mentioned agreement if you want to contribute to this project

Copilot AI changed the title [WIP] [Bug] Not possible to set inactivity_timeout in a Fleet policy Add inactivity_timeout support to Fleet agent policy resource Sep 8, 2025
Copilot AI requested a review from tobio September 8, 2025 00:33
Copilot finished work on behalf of tobio September 8, 2025 00:33
Copilot AI and others added 2 commits September 8, 2025 02:20
- Added MinVersionInactivityTimeout constant for 8.7.0
- Added SupportsInactivityTimeout field to features struct
- Updated buildFeatures to check for inactivity timeout support
- Added version validation in toAPICreateModel and toAPIUpdateModel
- Updated test to use proper version check for inactivity_timeout

Co-authored-by: tobio <[email protected]>
- Added unit tests to verify MinVersionInactivityTimeout constant
- Added comprehensive validation tests for version checking logic
- Verified version validation works for both create and update operations
- Ensured null/unset inactivity_timeout values don't trigger validation errors

Co-authored-by: tobio <[email protected]>
Copilot AI changed the title Add inactivity_timeout support to Fleet agent policy resource Add inactivity_timeout support to Fleet agent policy resource with version validation Sep 8, 2025
Copilot finished work on behalf of tobio September 8, 2025 02:25
Copilot AI requested a review from tobio September 8, 2025 02:25
Copilot AI requested a review from tobio September 8, 2025 07:42
Copilot finished work on behalf of tobio September 8, 2025 07:42
@tobio tobio marked this pull request as ready for review September 8, 2025 08:02
@tobio tobio requested review from a team, dimuon and nick-benoit September 8, 2025 08:02
Copy link
Contributor

@dimuon dimuon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides one nit-pick question, it looks good 👍 Also it makes sense to redefine the timeout as a string duration.

Copilot AI changed the title Add inactivity_timeout support to Fleet agent policy resource with version validation Add inactivity_timeout support to Fleet agent policy resource with duration string type and version validation Sep 10, 2025
Copilot finished work on behalf of tobio September 10, 2025 11:12
Copilot AI requested a review from tobio September 10, 2025 11:12
tobio

This comment was marked as outdated.

Copilot finished work on behalf of tobio September 10, 2025 11:28
Copilot AI requested a review from tobio September 10, 2025 11:28
@tobio tobio linked an issue Sep 10, 2025 that may be closed by this pull request
@tobio tobio merged commit 8d86de6 into main Sep 10, 2025
49 of 50 checks passed
@tobio tobio deleted the copilot/fix-641 branch September 10, 2025 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Not possible to set inactivity_timeout in a Fleet policy

4 participants