Skip to content

Conversation

@billyjbryant
Copy link

@billyjbryant billyjbryant commented Jul 23, 2025

Summary

  • Add panther_schema resource for custom log schema management
  • Add panther_user resource for user invitation and management
  • Add panther_role resource for role-based access control
  • Add panther_rule resource for detection rule management
  • Add panther_cloud_account resource for AWS cloud account integration
  • Replace hasura GraphQL client with machinebox for better schema support

New Resources

panther_schema

  • Create/read/update/delete custom log schemas
  • Support for YAML schema specifications with validation
  • Automatic field discovery and revision tracking

panther_user

  • Invite users with email and role assignment
  • Full CRUD operations for user management
  • Role association and status tracking

panther_role

  • Create custom roles with specific permissions
  • Permission validation and management
  • Full lifecycle management (create/read/update/delete)

panther_rule

  • Detection rule management with generated schema
  • Support for rule creation, updates, and deletion
  • Integration with Panther's detection engine

panther_cloud_account

  • AWS cloud account integration with exclusion features
  • Region and resource type filtering
  • Regex pattern exclusions for fine-grained control

GraphQL Client Migration

Replaced hasura/go-graphql-client with machinebox/graphql to resolve schema resource deployment issues:

  • Root Cause: hasura client had compatibility issues with Panther's GraphQL schema operations
  • Solution: machinebox client provides stricter field validation and better compatibility
  • Impact: All GraphQL resources now work reliably (100% acceptance test success rate)
  • Cleanup: Removed unused hasura dependency completely

Technical Implementation

  • All resources follow Terraform Plugin Framework patterns
  • Comprehensive acceptance test coverage
  • Proper import support for all resources
  • GraphQL and REST API integration as appropriate
  • Error handling and validation throughout

Test Results

ALL 7 RESOURCES PASSING (100% success rate):

  • HttpSourceResource
  • CloudAccountResource
  • RoleResource
  • RuleResource
  • S3SourceResource
  • SchemaResource
  • UserResource

🤖 Generated with Claude Code

@billyjbryant billyjbryant requested a review from a team as a code owner July 23, 2025 23:48
@CLAassistant
Copy link

CLAassistant commented Jul 23, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@billyjbryant billyjbryant changed the title Add Terraform resources for rules, roles, and users Add Terraform resources for cloud accounts, rules, roles, and users Jul 23, 2025
@billyjbryant
Copy link
Author

billyjbryant commented Jul 24, 2025

Issue

I encountered an issue in the handling of roles where the Terraform provider threw errors due to inconsistencies when roles included both *Read and *Modify permissions (e.g., LookupRead + LookupModify).

Root Cause

The Panther API implements permission inheritance, meaning *Modify permissions automatically include *Read access. This led to a few problems:

  1. Redundant Permission Filtering: The API was filtering out redundant *Read permissions (e.g., removing LookupRead when LookupModify existed).
  2. Inconsistent Ordering: The API was returning permissions in a different order than what was specified in our configuration.
  3. State Drift: This caused Terraform state to contain the filtered permissions, while the configuration still held the original, unfiltered permissions, leading to state drift.

Solution: Dual-Layer Permission Handling

I implemented a dual-layer permission handling approach to resolve this:

1. Input Filtering (filterInputPermissions)

  • What it does: Removes redundant *Read permissions before sending them to the API.
  • When it's applied: During Create and Update operations.
  • Benefit: Prevents the API from filtering permissions itself, ensuring consistency.

2. State Reconstitution (reconstitute)

  • What it does: Adds back the filtered *Read permissions into the Terraform state during Read operations.
  • Benefit: Ensures that the Terraform state accurately reflects the original user configuration, eliminating false drift detection.

Code Changes

The core changes are in internal/provider/resource_role.go:

  • Create/Update Operations: We now filter inputs before sending them to the API and store the original configuration in the state.
  • Read Operations: We get the API response, reconstitute the original permissions, and then store them in the state.

Result

  • ✅ Eliminates "Provider produced inconsistent result after apply" errors.
  • ✅ Users can specify permissions naturally (e.g., LookupRead + LookupModify).
  • ✅ Terraform state always matches the configuration.
  • ✅ The API receives compatible, filtered permissions.
  • ✅ No breaking changes to existing configurations.

@rleighton
Copy link

Thank you!

@billyjbryant billyjbryant changed the title Add Terraform resources for cloud accounts, rules, roles, and users Add Terraform resources for cloud accounts, rules, roles, users and schemas Jul 29, 2025
@billyjbryant
Copy link
Author

billyjbryant commented Jul 29, 2025

GraphQL Client Migration Update

What Changed:
Replaced with for all GraphQL operations.

Why This Was Necessary:
The original schema resource was failing with "Value Conversion Error" during deployment. The hasura client had compatibility issues with Panther's GraphQL schema operations, while machinebox provides stricter field validation and better compatibility.

Fixes Applied:

  • Field Names: Fixed mismatched fields (roles→role, removed non-existent picture/success fields)
  • Delete Operations: Updated to use __typename instead of non-existent success fields
  • S3Source Drift: Added missing fields to eliminate Terraform state drift
  • Response Parsing: Fixed GraphQL response structure for stricter validation

Result: ✅ 7/7 resources now pass acceptance tests (schema resource now works perfectly)

@markosfount
Copy link
Collaborator

Hi @billyjbryant, thanks for the contribution. I'll look into this PR in the next couple of days and come back to you!

schema:
ignores:
- id
policy:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some of these endpoints do not seem to be related with created resources using the rest-api, so they should not be included here. This file is for code generation of resources that leverage the rest-api, as described in the README

DeleteRule(ctx context.Context, id string) error


// Data Models management
Copy link
Collaborator

Choose a reason for hiding this comment

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

These methods also do not appear to be used for any resource, so they can be removed.

"context"
"encoding/json"
"fmt"
"github.com/hasura/go-graphql-client"
Copy link
Collaborator

@markosfount markosfount Aug 13, 2025

Choose a reason for hiding this comment

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

I understand you had issues with using the hasura client for schemas, but from what I can see the machinebox graphql client that is used instead is not an actively maintained repo, as the last release was 7 years ago. As such, I would prefer we don't switch to it in this repository. I haven't yet looked in detail at the issue you have mentioned, nor researched for alternatives, but combined with my general comment for splitting the PR to individual PRs for fewer resources, I think we can dive deeper into this when/if there is a dedicated PR for the graphql based resources.

Copy link
Author

Choose a reason for hiding this comment

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

Understood. I will admit that I didn't do my due diligence enough to vet the package's maintenance, I relied on searching for the errors I was seeing and finding multiple suggestions on the replacement package so I tested it. I will re-work the PR to split it up and focus largely on the rest APIs.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks @billyjbryant, for anything further please let us know.

@markosfount
Copy link
Collaborator

markosfount commented Aug 13, 2025

Hi @billyjbryant ,

Thanks a lot for the effort in creating this PR and the patience while we are reviewing it. I had a chat with folks from different teams as this PR covers resources that are owned by different functions of the company. The consensus is that all resources added in this PR are useful and a meaningful extension of the provider.

The PR, however, is quite big which makes it hard to review in a meaningful way and to test on our side that all resources will work as expected when handled with terraform. I'd like to suggest that it be broken down to PRs of 1-2 resources, which will make it easier to review and test. The PR could be broken down to individual PRs, depending on which resources you consider more useful or you would prefer to have in the provider sooner rather than later.

Additionally, one important remark here is that the direction we want the provider to take is to be based on the rest-api rather than the graph-api in the future. I can see that the resources you have added are split between the two. It should be noted that for the user and role resource there are available rest-api endpoints which can be leveraged instead of the graph-api that is used in this PR for these two resources. As such, I would suggest that the implementation uses the rest-api, which is a future-proof approach more in line with the planned direction for the provider.

Regarding resources that can only use the graphql api, after discussing with the team, while it's not in the direction we necessarily want to take, we are ok with adding those if customers consider that it will add value for them at this point. I would probably opt for adding those last (when splitting the PR up), unless they are the ones that you consider you need the most at this time. This would also give us some time to consider if we can add support for them in the rest-api in the meantime.

I have also left a couple high level comments to take into account when proceeding with individual PRs for resources.

Thanks again for the contribution, please let us know of any issues/comments on this.

@billyjbryant
Copy link
Author

@markosfount :

The PR, however, is quite big which makes it hard to review in a meaningful way and to test on our side that all resources will work as expected when handled with terraform. I'd like to suggest that it be broken down to PRs of 1-2 resources, which will make it easier to review and test. The PR could be broken down to individual PRs, depending on which resources you consider more useful or you would prefer to have in the provider sooner rather than later.

Agreed, I will break it down into smaller chunks to make it more manageable.

Additionally, one important remark here is that the direction we want the provider to take is to be based on the rest-api rather than the graph-api in the future. I can see that the resources you have added are split between the two. It should be noted that for the user and role resource there are available rest-api endpoints which can be leveraged instead of the graph-api that is used in this PR for these two resources. As such, I would suggest that the implementation uses the rest-api, which is a future-proof approach more in line with the planned direction for the provider.

Thank you, I wasn't sure of your intended direction and fell back to graphql for performance reasons. I will re-work to move things that do support rest api over to that instead :)

Regarding resources that can only use the graphql api, after discussing with the team, while it's not in the direction we necessarily want to take, we are ok with adding those if customers consider that it will add value for them at this point. I would probably opt for adding those last (when splitting the PR up), unless they are the ones that you consider you need the most at this time. This would also give us some time to consider if we can add support for them in the rest-api in the meantime.

Sounds good! Thank you!

Implements REST-based rule resource with full CRUD operations:

## New Resource Added:
- **panther_rule**: REST-based detection rule management using generated schema

## Key Features:
- Full CRUD operations (Create, Read, Update, Delete)
- Import support using rule IDs
- Generated schema from OpenAPI specification
- Support for all rule fields including tests, severity levels, log types
- Deduplication period and threshold configuration
- Tag and report management
- Runbook and summary attributes

## Client Implementation:
- Added Rule types to REST client interface
- Implemented REST endpoints at /rules
- Added Rule, CreateRuleInput, and UpdateRuleInput types
- Generic doRuleRequest helper for clean REST calls

## Code Generation:
- Updated generator_config.yml to include rule resource
- Updated provider-code-spec.json with rule schema
- Generated rule_resource_gen.go with complete schema validation

## Documentation:
- Added comprehensive docs/resources/rule.md
- Included detection-rules examples with main.tf, outputs.tf, rules.tf
- Added panther_rule resource example

## Testing:
- Included resource_rule_test.go for acceptance testing
- Provider successfully builds with new rule resource

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
billyjbryant and others added 2 commits October 9, 2025 16:45
Add Policy resource for managing Panther policies (cloud resource detection rules) using the REST API. Policies are similar to Rules but analyze cloud resources instead of logs.

Key changes:
- Add Policy types and REST methods to client
- Add generator config for policy, scheduled_rule, and simple_rule resources
- Implement panther_policy resource with CRUD operations
- Generate schema files for all detection resource types
- Manually add Id field to generated model structs (workaround for generator limitation)

Policies differ from Rules:
- Use ResourceTypes instead of LogTypes
- Simpler schema (no dedupPeriodMinutes, threshold, runbook, etc.)
- Include Suppressions field for ignoring specific resources

Also prepared infrastructure for scheduled_rule and simple_rule resources (types and client methods added, but resource implementations deferred to future work).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Adds two additional detection resource types to complement Rule and Policy:

- ScheduledRule: Detection rules that run on scheduled query results
  - Uses scheduled_queries instead of log_types
  - Analyzes aggregated data from scheduled queries
  - Includes dedup_period_minutes and threshold fields

- SimpleRule: YAML-based detection rules with no Python required
  - Uses detection field with YAML syntax instead of Python body
  - Compiles to Python automatically (read-only python_body field)
  - Includes SimpleRule-specific fields: alert_context, alert_title, dynamic_severities, group_by, inline_filters
  - Uses log_types like standard Rule

Also adds comprehensive test coverage for Policy, ScheduledRule, and SimpleRule resources with proper field validation based on actual schemas.

Includes documentation and examples demonstrating all four detection resource types with realistic use cases.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@billyjbryant
Copy link
Author

billyjbryant commented Oct 10, 2025

Closing this PR in favor of smaller, focused PRs as requested in the review feedback.

This PR has been split into three separate PRs:

Thank you for the guidance on breaking this up!

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.

4 participants