Skip to content

fix: Incompatible schema defined for mongodbatlas_backup_compliance_policy #1799

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jan 3, 2024
Merged

fix: Incompatible schema defined for mongodbatlas_backup_compliance_policy #1799

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
98a5703
chore: Add tfplugindocs & tfplugingen-openapi to the dev tools
andreaangiolillo Jan 2, 2024
fdff203
fix: incompatible schema defined for mongodbatlas_backup_compliance_p…
andreaangiolillo Jan 2, 2024
999e505
Update GNUmakefile
andreaangiolillo Jan 2, 2024
b4c9fa1
fix test
andreaangiolillo Jan 2, 2024
21c34c2
Merge remote-tracking branch 'origin/master' into CLOUDP-220384
andreaangiolillo Jan 2, 2024
ec7f2db
Merge remote-tracking branch 'origin/master' into CLOUDP-220384
andreaangiolillo Jan 3, 2024
be074ec
addressed PR comment
andreaangiolillo Jan 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions internal/service/backupcompliancepolicy/data_source_backup_compliance_policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ func DataSource() *schema.Resource {
},
"on_demand_policy_item": {
Type: schema.TypeList,
MaxItems: 1,
Optional: true,
Computed: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

any potential breaking changes ? or plan change detections?
I am asking to make sure you've done some extra testing.

Copy link
Member

Choose a reason for hiding this comment

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

while I do not see a risk in this change as project_id is documented as the only argument for the data source, we should consider also fixing inconsistencies present in policy_item_hourly, policy_item_daily, policy_item_weekly and policy_item_monthly which are not defined as computed. Error seems to only be raised in on_demand_policy_item because all nested attributes are computed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@AgustinBettati All the policy_item_* fields have a few required parameters such as retention_unit and retention_value which is the reason why we are not getting the error for them.

Copy link
Collaborator Author

@andreaangiolillo andreaangiolillo Jan 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@marcosuma I was able to test locally that this is not a breaking change.
Testing scenario:

  • Run the following configuration
resource "mongodbatlas_project" "atlas-project" {
  name   = "test-andrea"
  org_id = "XXXXXXXXXXXXXX"
}

resource "mongodbatlas_backup_compliance_policy" "backup_policy" {
  project_id                 = mongodbatlas_project.atlas-project.id
  authorized_email           = "[email protected]"
  authorized_user_first_name = "ssss"
  authorized_user_last_name  = "sss"
  copy_protection_enabled    = false
  pit_enabled                = false
  encryption_at_rest_enabled = false

  restore_window_days = 7

  on_demand_policy_item {
    frequency_interval = 1
    retention_unit     = "days"
    retention_value    = 3
  }

  policy_item_hourly {
    frequency_interval = 1
    retention_unit     = "days"
    retention_value    = 7
  }

  policy_item_daily {
    frequency_interval = 1
    retention_unit     = "days"
    retention_value    = 7
  }

  policy_item_weekly {
    frequency_interval = 1
    retention_unit     = "weeks"
    retention_value    = 4
  }

  policy_item_monthly {
    frequency_interval = 1
    retention_unit     = "months"
    retention_value    = 12
  }

}

data "mongodbatlas_backup_compliance_policy" "backup_policy" {
  project_id = mongodbatlas_backup_compliance_policy.backup_policy.project_id
}
  • Run TF apply with the latest provider
Apply complete! Resources: 2 added, 0 changed, 0 destroyed.
  • Updated the provider to use local version and run again terraform apply
No changes. Your infrastructure matches the configuration.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Copy link
Member

Choose a reason for hiding this comment

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

Just to be sure, is it possible that the relevant data source backup_compliance_policy was not tested in the above test configuration?

All the policy_item_* fields have a few required parameters such as retention_unit and retention_value which is the reason why we are not getting the error for them

Correct, I was bringing this up as I don't see why there should be optional or required attributes given that they should not be defined in the configuration of the data source. All of them should be computed except for project_id which is the only argument for using the data source.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just to be sure, is it possible that the relevant data source backup_compliance_policy was not tested in the above test configuration?

ops, I copied/pasted the wrong conf 🤦 I updated my message. Thanks!

Correct, I was bringing this up as I don't see why there should be optional or required attributes given that they should not be defined in the configuration of the data source. All of them should be computed except for project_id which is the only argument for using the data source.

Thanks for the clarification. I agree, I updated the schema to make them computed as well. I also retested the new changes with the TF conf and no changes where detected. Thanks!

Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"id": {
Expand Down