-
Notifications
You must be signed in to change notification settings - Fork 92
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
Adding a computed SingleNestedAttribute to a resource causes other fields show (known after apply) #599
Comments
@iwarapter thanks for reporting this and sorry you're running into trouble here. For additional context, the framework's plan handling is as follows:
Note that schema-based plan modification doesn't occur before the unknown marking. This is being tracked in #183, but didn't land before v1.0.0 because it could have some significant impacts to how plan modifications are coded in provider logic. Using any sort of "default value" plan modifier likely causes the unknown marking to always happen. For the most part, the framework is relying on Terraform to manage the merging of prior state and configuration data into a proposed new state, but since the proposed new state for an attribute without a configuration value would be null (while the prior state is not null) the unknown marking will always be triggered. Given the human readable plan here, it might be good to figure out the expected semantics of the It also may be a good time to rule out a case of Terraform's human plan renderer unexpectedly hiding some value change details, which we are generally tracking in hashicorp/terraform#31887. One triage action we can use for that is view the JSON output of the plan, since it will contain all the underlying value details. For example [
{
"address": "time_rotating.test",
"mode": "managed",
"type": "time_rotating",
"name": "test",
"provider_name": "registry.terraform.io/hashicorp/time",
"change": {
"actions": [
"update"
],
"before": {
"day": 6,
"hour": 17,
"id": "2023-01-05T17:35:42Z",
"minute": 35,
"month": 1,
"rfc3339": "2023-01-05T17:35:42Z",
"rotation_days": 1,
"rotation_hours": null,
"rotation_minutes": null,
"rotation_months": null,
"rotation_rfc3339": "2023-01-06T17:35:42Z",
"rotation_years": null,
"second": 42,
"triggers": null,
"unix": 1673026542,
"year": 2023
},
"after": {
"day": 7,
"hour": 17,
"id": "2023-01-05T17:35:42Z",
"minute": 35,
"month": 1,
"rfc3339": "2023-01-05T17:35:42Z",
"rotation_days": 2,
"rotation_hours": null,
"rotation_minutes": null,
"rotation_months": null,
"rotation_rfc3339": "2023-01-07T17:35:42Z",
"rotation_years": null,
"second": 42,
"triggers": null,
"unix": 1673112942,
"year": 2023
},
"after_unknown": {},
"before_sensitive": {},
"after_sensitive": {}
}
}
] Hopefully checking all the You can setup your Terraform CLI configuration file with Another option would be to textually diff the Terraform state between two applies. It is also possible that attempting the second apply might raise a Terraform error with additional clues. |
So I have created the json extract: [
{
"address": "pingfederate_oauth_client.example1",
"mode": "managed",
"type": "pingfederate_oauth_client",
"name": "example1",
"provider_name": "registry.terraform.io/iwarapter/pingfederate",
"change": {
"actions": [
"update"
],
"before": {
"allow_authentication_api_init": false,
"bypass_activation_code_confirmation_override": null,
"bypass_approval_page": false,
"ciba_delivery_mode": null,
"ciba_notification_endpoint": null,
"ciba_polling_interval": null,
"ciba_request_object_signing_algorithm": null,
"ciba_require_signed_requests": null,
"ciba_user_code_supported": null,
"client_auth": null,
"client_id": "tf-acc-woot",
"client_secret_changed_time": null,
"client_secret_retention_period": null,
"client_secret_retention_period_type": "SERVER_DEFAULT",
"default_access_token_manager_ref": "testme",
"description": null,
"device_flow_setting_type": "SERVER_DEFAULT",
"device_polling_interval_override": null,
"enabled": true,
"exclusive_scopes": [],
"extended_parameters": null,
"grant_types": [
"EXTENSION"
],
"id": "tf-acc-woot",
"jwks_settings": null,
"jwt_secured_authorization_response_mode_content_encryption_algorithm": null,
"jwt_secured_authorization_response_mode_encryption_algorithm": null,
"jwt_secured_authorization_response_mode_signing_algorithm": null,
"logo_url": null,
"name": "my-client-name",
"oidc_policy": {
"grant_access_session_revocation_api": false,
"grant_access_session_session_management_api": false,
"id_token_content_encryption_algorithm": null,
"id_token_encryption_algorithm": null,
"id_token_signing_algorithm": "RS256",
"logout_uris": [
"https://logout"
],
"pairwise_identifier_user_type": false,
"ping_access_logout_capable": true,
"policy_group": null,
"sector_identifier_uri": null
},
"pending_authorization_timeout_override": null,
"persistent_grant_expiration_time": 0,
"persistent_grant_expiration_time_unit": "DAYS",
"persistent_grant_expiration_type": "SERVER_DEFAULT",
"persistent_grant_idle_timeout": 0,
"persistent_grant_idle_timeout_time_unit": "DAYS",
"persistent_grant_idle_timeout_type": "SERVER_DEFAULT",
"persistent_grant_reuse_grant_types": null,
"persistent_grant_reuse_type": "SERVER_DEFAULT",
"redirect_uris": [],
"refresh_rolling": "SERVER_DEFAULT",
"refresh_token_rolling_grace_period": null,
"refresh_token_rolling_grace_period_type": "SERVER_DEFAULT",
"refresh_token_rolling_interval": null,
"refresh_token_rolling_interval_type": "SERVER_DEFAULT",
"request_object_signing_algorithm": null,
"request_policy_ref": null,
"require_jwt_secured_authorization_response_mode": false,
"require_proof_key_for_code_exchange": false,
"require_pushed_authorization_requests": false,
"require_signed_requests": false,
"restrict_scopes": false,
"restrict_to_default_access_token_manager": false,
"restricted_response_types": [],
"restricted_scopes": [],
"token_exchange_processor_policy_ref": null,
"token_introspection_content_encryption_algorithm": null,
"token_introspection_encryption_algorithm": null,
"token_introspection_signing_algorithm": null,
"user_authorization_url_override": null,
"validate_using_all_eligible_atms": false
},
"after": {
"allow_authentication_api_init": false,
"bypass_activation_code_confirmation_override": null,
"bypass_approval_page": false,
"ciba_delivery_mode": null,
"ciba_notification_endpoint": null,
"ciba_polling_interval": null,
"ciba_request_object_signing_algorithm": null,
"ciba_require_signed_requests": null,
"ciba_user_code_supported": null,
"client_auth": null,
"client_id": "tf-acc-woot",
"client_secret_retention_period": null,
"client_secret_retention_period_type": "SERVER_DEFAULT",
"default_access_token_manager_ref": "testme",
"description": null,
"device_flow_setting_type": "SERVER_DEFAULT",
"device_polling_interval_override": null,
"enabled": true,
"exclusive_scopes": [],
"extended_parameters": null,
"grant_types": [
"EXTENSION"
],
"id": "tf-acc-woot",
"jwks_settings": null,
"jwt_secured_authorization_response_mode_content_encryption_algorithm": null,
"jwt_secured_authorization_response_mode_encryption_algorithm": null,
"jwt_secured_authorization_response_mode_signing_algorithm": null,
"logo_url": null,
"name": "my-client-name",
"oidc_policy": {
"grant_access_session_revocation_api": false,
"grant_access_session_session_management_api": false,
"id_token_content_encryption_algorithm": null,
"id_token_encryption_algorithm": null,
"id_token_signing_algorithm": "RS256",
"logout_uris": [
"https://logout"
],
"pairwise_identifier_user_type": false,
"ping_access_logout_capable": true,
"policy_group": null,
"sector_identifier_uri": null
},
"pending_authorization_timeout_override": null,
"persistent_grant_expiration_time": 0,
"persistent_grant_expiration_time_unit": "DAYS",
"persistent_grant_expiration_type": "SERVER_DEFAULT",
"persistent_grant_idle_timeout": 0,
"persistent_grant_idle_timeout_time_unit": "DAYS",
"persistent_grant_idle_timeout_type": "SERVER_DEFAULT",
"persistent_grant_reuse_grant_types": null,
"persistent_grant_reuse_type": "SERVER_DEFAULT",
"redirect_uris": [],
"refresh_rolling": "SERVER_DEFAULT",
"refresh_token_rolling_grace_period": null,
"refresh_token_rolling_grace_period_type": "SERVER_DEFAULT",
"refresh_token_rolling_interval": null,
"refresh_token_rolling_interval_type": "SERVER_DEFAULT",
"request_object_signing_algorithm": null,
"request_policy_ref": null,
"require_jwt_secured_authorization_response_mode": false,
"require_proof_key_for_code_exchange": false,
"require_pushed_authorization_requests": false,
"require_signed_requests": false,
"restrict_scopes": false,
"restrict_to_default_access_token_manager": false,
"restricted_response_types": [],
"restricted_scopes": [],
"token_exchange_processor_policy_ref": null,
"token_introspection_content_encryption_algorithm": null,
"token_introspection_encryption_algorithm": null,
"token_introspection_signing_algorithm": null,
"user_authorization_url_override": null,
"validate_using_all_eligible_atms": false
},
"after_unknown": {
"client_secret_changed_time": true,
"exclusive_scopes": [],
"grant_types": [
false
],
"oidc_policy": {
"logout_uris": [
false
]
},
"redirect_uris": [],
"restricted_response_types": [],
"restricted_scopes": []
},
"before_sensitive": {
"exclusive_scopes": [],
"grant_types": [
false
],
"oidc_policy": {
"logout_uris": [
false
]
},
"redirect_uris": [],
"restricted_response_types": [],
"restricted_scopes": []
},
"after_sensitive": {
"exclusive_scopes": [],
"grant_types": [
false
],
"oidc_policy": {
"logout_uris": [
false
]
},
"redirect_uris": [],
"restricted_response_types": [],
"restricted_scopes": []
}
}
}
] |
So the |
hi @bflad thanks for your tips above, this is where I was able to get to: Client with no secret but resource "pingfederate_oauth_client" "example1" {
client_id = "tf-acc-secret1"
name = "my-client-name"
grant_types = ["EXTENSION"]
default_access_token_manager_ref = "testme"
oidc_policy = {
grant_access_session_revocation_api = false
ping_access_logout_capable = true
}
} After Apply Plan
Now if I don't define the resource "pingfederate_oauth_client" "example1" {
client_id = "tf-acc-secret1"
name = "my-client-name"
grant_types = ["EXTENSION"]
default_access_token_manager_ref = "testme"
# oidc_policy = {
# grant_access_session_revocation_api = false
# ping_access_logout_capable = true
# }
} Then no changes,
Similar behaviour happens if i define a client with a secret but this time on the resource "pingfederate_oauth_client" "example1" {
client_id = "tf-acc-secret1"
name = "my-client-name"
grant_types = ["EXTENSION"]
default_access_token_manager_ref = "testme"
client_auth = {
type = "SECRET"
secret = "top_secret"
}
oidc_policy = {
grant_access_session_revocation_api = false
ping_access_logout_capable = false
}
}
And without the resource "pingfederate_oauth_client" "example1" {
client_id = "tf-acc-secret1"
name = "my-client-name"
grant_types = ["EXTENSION"]
default_access_token_manager_ref = "testme"
client_auth = {
type = "SECRET"
secret = "top_secret"
}
# oidc_policy = {
# grant_access_session_revocation_api = false
# ping_access_logout_capable = false
# }
}
I can solve the client_secret_changed_time with a resource func (r *pingfederateOAuthClientResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
var data ClientData
diags := req.Plan.Get(ctx, &data)
if diags.HasError() {
return
}
if data.ClientAuth != nil {
if data.ClientAuth.Secret.IsNull() {
data.ClientSecretChangedTime = types.StringNull()
}
} else {
data.ClientSecretChangedTime = types.StringNull()
}
resp.Diagnostics = append(resp.Diagnostics, resp.Plan.Set(ctx, &data)...) But haven't managed to do the same for the encrypted secret yet. All of this stems from the new default object so i'm still unsure why everything that was previously working, now isn't. |
I have just tried setting the default through the resource |
I think someone else has the same problem https://discuss.hashicorp.com/t/singlenestedattribute-plan-detects-unexisting-changes/48138/2 |
hey @bflad, anything else you need to help diagnose this? |
My understanding of this is as follows:
The rub here is that a single nested attribute that is partially configured, will disable Terraform core from copying anything in the prior state the proposed new state for the entire attribute (copying the configuration value instead), which means the there will always be a difference between the prior state and proposed new state according to the framework. This is currently unavoidable at the moment. There are at least three paths to follow here:
The last option would likely be providing native "default value" functionality, so the proposed new state can get "fixed" to match prior state in these cases, but there is some design work to be done there. |
hey @bflad, thanks for the detailed info, yup your understanding is correct.
Sounds like this is my only option (short term), how do I go about this? My previous attempts to match the expected end results (using resource |
It turns out that for
We might make some headway on this after discussing (the very similar to this issue) hashicorp/terraform#32522. This would mean a Terraform update could potentially alleviate parts of this issue. I'd subscribe there for further updates on that front.
You're in an awkward position because the framework is making decisions based on assumptions about the values its receiving from Terraform. While potential changes in Terraform could help, there's also no way to opt out of the current framework unknown value marking behavior because it happens before any potential provider-definable logic. Given that, one potential option here would be to fix up things based on the current situation by:
These two provider-defined plan modification steps run in exactly that order -- schema (best to assume randomized ordering between attributes) then resource-level. We can at least try to use that to our advantage here. It's not pretty and I haven't tested this for validity, but the // Please note that this example assumes only encrypted_secret is an issue.
// Other Computed-only attributes without UseStateForUnknown would need
// to potentially be copied from prior state to try and recreate a
// "no change" plan for comparison.
func (r *pingfederateOAuthClientResource) ModifyPlan(ctx context.Context, req resource.ModifyPlanRequest, resp *resource.ModifyPlanResponse) {
// Skip modification on resource creation
if req.State.Raw.IsNull() {
return
}
// Skip modification on resource destruction
if req.Plan.Raw.IsNull() {
return
}
// Skip modification if planning no changes
if req.Plan.Raw.Equal(req.State.Raw) {
return
}
var originalPlan, plan, state ClientData
resp.Diagnostics.Append(req.Plan.Get(ctx, &originalPlan)...)
resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...)
resp.Diagnostics.Append(req.State.Get(ctx, &state)...)
if resp.Diagnostics.HasError() {
return
}
// Skip modification if encrypted_secret is known
if !plan.ClientAuth.EncryptedSecret.IsUnknown() {
return
}
// Copy encrypted_secret prior state to plan
plan.ClientAuth.EncryptedSecret = state.ClientAuth.EncryptedSecret
resp.Diagnostics.Append(resp.Plan.Set(ctx, &plan)...)
if resp.Diagnostics.HasError() {
return
}
// Undo modification if there are changes outside encrypted_secret
if !resp.Plan.Raw.Equal(req.State.Raw) {
resp.Diagnostics.Append(resp.Plan.Set(ctx, &originalPlan)...)
}
} Hopefully the combination of the Terraform fix and allowing providers to move default value logic before the unknown marking should make this type of workaround unnecessary in the future. |
Hi again, @iwarapter 👋 It's been awhile since we checked in and in the meantime the framework implemented native support for default attribute values. Are you still having issues after using that more recent functionality? |
Closing due to lack of response and no other interest or commentary. If you are still having trouble, please open a new issue and feel free to reference this issue. 👍 |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Module version
Relevant provider source code
This is adding the following diff to iwarapter/terraform-provider-pingfederate@bb0e607
The resource is defined: https://github.com/iwarapter/terraform-provider-pingfederate/blob/bb0e6075159d55a549b51ea7c884a255d9b3ccdf/internal/framework/schemes.go#L192-L643
Terraform Configuration Files
Debug Output
Expected Behavior
When the default plan modifier is added to the object, defaults are provided.
Actual Behavior
When the
oidc_policy
default Plan Modifier is added in, it causes other attributes to show as changing:Steps to Reproduce
With the plan modifier added tests fail: iwarapter/terraform-provider-pingfederate#252
Here is a gist of the trace for the tests https://gist.github.com/iwarapter/8dc5a723e14e83e40e4280f928c7f236
References
#582
The text was updated successfully, but these errors were encountered: