Skip to content
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

PRC failures in AWS #4410

Closed
2 of 6 tasks
VenelinMartinov opened this issue Aug 27, 2024 · 7 comments · Fixed by #4415
Closed
2 of 6 tasks

PRC failures in AWS #4410

VenelinMartinov opened this issue Aug 27, 2024 · 7 comments · Fixed by #4415
Assignees
Labels
kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed

Comments

@VenelinMartinov
Copy link
Contributor

VenelinMartinov commented Aug 27, 2024

Describe what happened

Multiple test failures when enabling PRC in AWS:

related to pulumi/pulumi-terraform-bridge#1785

Sample program

#4403

Log output

No response

Affected Resource(s)

No response

Output of pulumi about

.

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@VenelinMartinov VenelinMartinov added the kind/engineering Work that is not visible to an external user label Aug 27, 2024
@pulumi-bot pulumi-bot added the needs-triage Needs attention from the triage team label Aug 27, 2024
@VenelinMartinov VenelinMartinov removed the needs-triage Needs attention from the triage team label Aug 27, 2024
@t0yv0
Copy link
Member

t0yv0 commented Aug 27, 2024

Tags might need normTags from https://github.com/pulumi/pulumi-aws/pull/4013/files#diff-2e4fffe32720b3d310b4336cfdde6519ab1bf6d9e0d82b418299817fdf42a29fR205

RDSInstance was I recall a problem with moot test setup, might need workaround from 4013 as well.

@t0yv0
Copy link
Member

t0yv0 commented Aug 27, 2024

The rest look new to me.

@VenelinMartinov
Copy link
Contributor Author

VenelinMartinov commented Aug 28, 2024

TestAccDefaultTags seems to be an issue with importing the legacy bucket. After importing the generated code contains some defaults not present in the original:

{
    "method": "/pulumirpc.ResourceProvider/Diff",
    "request": {
        "id": "res-771212f",
        "urn": "urn:pulumi:test::test-aws-legacy::aws:s3/bucket:Bucket::res",
        "olds": {
            "__meta": "{\"schema_version\":\"0\"}",
            "accelerationStatus": "",
            "acl": null,
            "arn": "arn:aws:s3:::res-771212f",
            "bucket": "res-771212f",
            "bucketDomainName": "res-771212f.s3.amazonaws.com",
            "bucketPrefix": null,
            "bucketRegionalDomainName": "res-771212f.s3.us-west-2.amazonaws.com",
            "corsRules": [],
            "forceDestroy": null,
            "grants": [],
            "hostedZoneId": "Z3BJ6K6RIION7M",
            "id": "res-771212f",
            "lifecycleRules": [],
            "loggings": [],
            "objectLockConfiguration": null,
            "policy": null,
            "region": "us-west-2",
            "replicationConfiguration": null,
            "requestPayer": "BucketOwner",
            "serverSideEncryptionConfiguration": {
                "rule": {
                    "applyServerSideEncryptionByDefault": {
                        "kmsMasterKeyId": "",
                        "sseAlgorithm": "AES256"
                    },
                    "bucketKeyEnabled": false
                }
            },
            "tags": {
                "GlobalTag": "bar",
                "LocalTag": "foo"
            },
            "tagsAll": {
                "GlobalTag": "bar",
                "LocalTag": "foo"
            },
            "versioning": {
                "enabled": false,
                "mfaDelete": false
            },
            "website": null,
            "websiteDomain": null,
            "websiteEndpoint": null
        },
        "news": {
            "__defaults": [
                "acl",
                "bucket",
                "forceDestroy"
            ],
            "acl": "private",
            "bucket": "res-771212f",
            "forceDestroy": false,
            "tags": {
                "GlobalTag": "bar",
                "LocalTag": "foo"
            },
            "tagsAll": {
                "GlobalTag": "bar",
                "LocalTag": "foo"
            }
        },
        "oldInputs": {
            "__defaults": [],
            "acl": null,
            "arn": "arn:aws:s3:::res-771212f",
            "bucket": "res-771212f",
            "forceDestroy": null,
            "hostedZoneId": "Z3BJ6K6RIION7M",
            "requestPayer": "BucketOwner",
            "serverSideEncryptionConfiguration": {
                "__defaults": [],
                "rule": {
                    "__defaults": [],
                    "applyServerSideEncryptionByDefault": {
                        "__defaults": [],
                        "sseAlgorithm": "AES256"
                    }
                }
            },
            "tags": {
                "GlobalTag": "bar",
                "LocalTag": "foo",
                "__defaults": []
            },
            "tagsAll": {
                "GlobalTag": "bar",
                "LocalTag": "foo",
                "__defaults": []
            }
        }
    },
    "response": {
        "stables": [
            "bucket",
            "bucketPrefix"
        ],
        "changes": "DIFF_SOME",
        "diffs": [
            "acl",
            "forceDestroy"
        ],
        "detailedDiff": {
            "acl": {},
            "forceDestroy": {}
        },
        "hasDetailedDiff": true
    },
    "metadata": {
        "kind": "resource",
        "mode": "client",
        "name": "aws"
    }
}

The diff seems legitimate - the properties are non-computed and not set in the initial up but are returned by Read.

@VenelinMartinov
Copy link
Contributor Author

VenelinMartinov commented Aug 28, 2024

TestEKSClusterUpgrade might require some intervention - it has had a default added for a parameter which has ForceNew:

{
    "method": "/pulumirpc.ResourceProvider/Diff",
    "request": {
        "id": "tested-resource-0268e11",
        "urn": "urn:pulumi:test::test::aws:eks/cluster:Cluster::tested-resource",
        "olds": {
            "__meta": "{\"e2bfb730-ecaa-11e6-8f88-34363bc7c4c0\":{\"create\":1800000000000,\"delete\":900000000000,\"update\":3600000000000}}",
            "arn": "arn:aws:eks:us-west-2:616138583583:cluster/tested-resource-0268e11",
            "certificateAuthorities": [
                {
                    "data": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURCVENDQWUyZ0F3SUJBZ0lJSUovTzc1dkYzSEl3RFFZSktvWklodmNOQVFFTEJRQXdGVEVUTUJFR0ExVUUKQXhNS2EzVmlaWEp1WlhSbGN6QWVGdzB5TXpFd01EVXlNVFE0TlRkYUZ3MHpNekV3TURJeU1UUTROVGRhTUJVeApFekFSQmdOVkJBTVRDbXQxWW1WeWJtVjBaWE13Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLCkFvSUJBUURQT0ZiVGFnbUY2d2Y5SWs3L0d0VEdmU09VaVIwQUo1aDZUUktEcXk1NE9xWFJVQVpCNEwxTDZvYnkKVHdHLzFaNGJlM3U1bm9GdUVZdUEyTEltRlhKTVZ1eHI0OXV6U1BsWkk2eXZuZTVaWkVkZG5aclVWTjl4U00rdQpjakxrZEYwRXh6SnIrU1BZaXphQVQweG44YUtRRXlROHZvZm1zcml4YWJpY0ZwSW1TdmF3OWdMZnBpMXZwYXRwClJraW1NK202a1lWeGFZS1dUUUJBbnRVTzk3MXlWbVJvR1ZkOVZscU0rQ2NPS0svQml4NG5qYzNXNFZjWnZDZEcKMjVmei9hR2trdFA2SkxONnJ0TUlUN3ZWNkhXWklhMU1Sak85cmR4OXFqVVJMT3Z3akhnLytyZkhTQTR1ZDlxYQpOUjZxZUhSMUlTN1RJdmZzRlF3Z3p1WDFrRlh4QWdNQkFBR2pXVEJYTUE0R0ExVWREd0VCL3dRRUF3SUNwREFQCkJnTlZIUk1CQWY4RUJUQURBUUgvTUIwR0ExVWREZ1FXQkJTYno4SjRVck9Fam8xcHl0OFZPenVKakVIbGdUQVYKQmdOVkhSRUVEakFNZ2dwcmRXSmxjbTVsZEdWek1BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQkRJT0ZlWGRIVApUZmhUQlAyYlFpSkVDa24xenk5NTBpQTZUM3RsajBPOW9tTDVRUHB5dTVXWHkxRUlkbkNIdkl6QVl1RkhxZ0swClVLUFgxdURmOG02QWJPdUxteFJBNFFpc2NrNWJBQmN6Q1lienhJM3NGRkUzQ2hONHBpblB5QVhoUUFJd1FFUXkKVzNSUGQ0Mlh5WXVKTTRITTBJS0RDZ2dCNGhEWEtQRFlPWHQ3a0Zab1I4MmNCaVpVa3BqUUpSUG9Yd0ZCcElYcQoxczN3U1JHZTRRSW5OWHdMQlpCMTVaM01lYkpkU01MQWloRUpTY3J5MWRhZXhrQ1dTMUMrY1kxN2FWaEhld0pGCmx2am1NSzJxRTl4Rm85RmgzV2xTbEVnc3FZU0dNUXFxc2RlNmtYNDVlYzBRM1dab0pEZGtnZ3BwckJ2eGV2aGsKVnV0MFJ3QmJTS2pyCi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"
                }
            ],
            "certificateAuthority": {
                "data": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSURCVENDQWUyZ0F3SUJBZ0lJSUovTzc1dkYzSEl3RFFZSktvWklodmNOQVFFTEJRQXdGVEVUTUJFR0ExVUUKQXhNS2EzVmlaWEp1WlhSbGN6QWVGdzB5TXpFd01EVXlNVFE0TlRkYUZ3MHpNekV3TURJeU1UUTROVGRhTUJVeApFekFSQmdOVkJBTVRDbXQxWW1WeWJtVjBaWE13Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLCkFvSUJBUURQT0ZiVGFnbUY2d2Y5SWs3L0d0VEdmU09VaVIwQUo1aDZUUktEcXk1NE9xWFJVQVpCNEwxTDZvYnkKVHdHLzFaNGJlM3U1bm9GdUVZdUEyTEltRlhKTVZ1eHI0OXV6U1BsWkk2eXZuZTVaWkVkZG5aclVWTjl4U00rdQpjakxrZEYwRXh6SnIrU1BZaXphQVQweG44YUtRRXlROHZvZm1zcml4YWJpY0ZwSW1TdmF3OWdMZnBpMXZwYXRwClJraW1NK202a1lWeGFZS1dUUUJBbnRVTzk3MXlWbVJvR1ZkOVZscU0rQ2NPS0svQml4NG5qYzNXNFZjWnZDZEcKMjVmei9hR2trdFA2SkxONnJ0TUlUN3ZWNkhXWklhMU1Sak85cmR4OXFqVVJMT3Z3akhnLytyZkhTQTR1ZDlxYQpOUjZxZUhSMUlTN1RJdmZzRlF3Z3p1WDFrRlh4QWdNQkFBR2pXVEJYTUE0R0ExVWREd0VCL3dRRUF3SUNwREFQCkJnTlZIUk1CQWY4RUJUQURBUUgvTUIwR0ExVWREZ1FXQkJTYno4SjRVck9Fam8xcHl0OFZPenVKakVIbGdUQVYKQmdOVkhSRUVEakFNZ2dwcmRXSmxjbTVsZEdWek1BMEdDU3FHU0liM0RRRUJDd1VBQTRJQkFRQkRJT0ZlWGRIVApUZmhUQlAyYlFpSkVDa24xenk5NTBpQTZUM3RsajBPOW9tTDVRUHB5dTVXWHkxRUlkbkNIdkl6QVl1RkhxZ0swClVLUFgxdURmOG02QWJPdUxteFJBNFFpc2NrNWJBQmN6Q1lienhJM3NGRkUzQ2hONHBpblB5QVhoUUFJd1FFUXkKVzNSUGQ0Mlh5WXVKTTRITTBJS0RDZ2dCNGhEWEtQRFlPWHQ3a0Zab1I4MmNCaVpVa3BqUUpSUG9Yd0ZCcElYcQoxczN3U1JHZTRRSW5OWHdMQlpCMTVaM01lYkpkU01MQWloRUpTY3J5MWRhZXhrQ1dTMUMrY1kxN2FWaEhld0pGCmx2am1NSzJxRTl4Rm85RmgzV2xTbEVnc3FZU0dNUXFxc2RlNmtYNDVlYzBRM1dab0pEZGtnZ3BwckJ2eGV2aGsKVnV0MFJ3QmJTS2pyCi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"
            },
            "createdAt": "2023-10-05 21:44:26.403 +0000 UTC",
            "enabledClusterLogTypes": [],
            "encryptionConfig": null,
            "endpoint": "https://89180AEA318B417FC070BC1183C35AB4.gr7.us-west-2.eks.amazonaws.com",
            "id": "tested-resource-0268e11",
            "identities": [
                {
                    "oidcs": [
                        {
                            "issuer": "https://oidc.eks.us-west-2.amazonaws.com/id/89180AEA318B417FC070BC1183C35AB4"
                        }
                    ]
                }
            ],
            "kubernetesNetworkConfig": {
                "ipFamily": "ipv4",
                "serviceIpv4Cidr": "172.20.0.0/16",
                "serviceIpv6Cidr": ""
            },
            "name": "tested-resource-0268e11",
            "outpostConfig": null,
            "platformVersion": "eks.5",
            "roleArn": "arn:aws:iam::616138583583:role/ekscluster1role-6da3f8e",
            "status": "ACTIVE",
            "tags": {},
            "tagsAll": {},
            "version": "1.27",
            "vpcConfig": {
                "clusterSecurityGroupId": "sg-0845b6841e1f4157c",
                "endpointPrivateAccess": false,
                "endpointPublicAccess": true,
                "publicAccessCidrs": [
                    "0.0.0.0/0"
                ],
                "securityGroupIds": [],
                "subnetIds": [
                    "subnet-0569afbec481e91ea",
                    "subnet-01397ef1aa6010338"
                ],
                "vpcId": "vpc-08e15731978bce209"
            }
        },
        "news": {
            "__defaults": [
                "bootstrapSelfManagedAddons",
                "name"
            ],
            "bootstrapSelfManagedAddons": true,
            "name": "tested-resource-0268e11",
            "roleArn": "arn:aws:iam::616138583583:role/ekscluster1role-6da3f8e",
            "vpcConfig": {
                "__defaults": [
                    "endpointPrivateAccess",
                    "endpointPublicAccess"
                ],
                "endpointPrivateAccess": false,
                "endpointPublicAccess": true,
                "subnetIds": [
                    "subnet-01397ef1aa6010338",
                    "subnet-0569afbec481e91ea"
                ]
            }
        },
        "oldInputs": {
            "__defaults": [
                "name"
            ],
            "name": "tested-resource-0268e11",
            "roleArn": "arn:aws:iam::616138583583:role/ekscluster1role-6da3f8e",
            "vpcConfig": {
                "__defaults": [
                    "endpointPrivateAccess",
                    "endpointPublicAccess"
                ],
                "endpointPrivateAccess": false,
                "endpointPublicAccess": true,
                "subnetIds": [
                    "subnet-01397ef1aa6010338",
                    "subnet-0569afbec481e91ea"
                ]
            }
        }
    },
    "response": {
        "replaces": [
            "bootstrapSelfManagedAddons"
        ],
        "stables": [
            "name",
            "roleArn"
        ],
        "changes": "DIFF_SOME",
        "diffs": [
            "bootstrapSelfManagedAddons"
        ],
        "detailedDiff": {
            "bootstrapSelfManagedAddons": {
                "kind": "ADD_REPLACE"
            }
        },
        "hasDetailedDiff": true
    },
    "metadata": {
        "kind": "resource",
        "mode": "client",
        "name": "aws"
    }
}

We had similar issues with pulumi-azure - this likely repores in TF under -refrash=false but not under -refresh=true.

The workaround there was to add a TransformFromState which adds the default value if not present in the state.

Raised #4415

EDIT: As @flostadler correctly pointed out, the TF provider actually handles this one with a migration. Unfortunately we do not run it due to pulumi/pulumi-terraform-bridge#2039. Fixed with #4416

@VenelinMartinov
Copy link
Contributor Author

VenelinMartinov commented Aug 28, 2024

TestIMDSAuth - the failure does not seem to repro locally. Could be a flake?

TestOpenZfsFileSystemUpgrade - this looks like it needed a re-recording. Still failing in CI but not locally.

TestRdsParameterGroupUnclearDiff does not seem to repro locally either.

@VenelinMartinov
Copy link
Contributor Author

The EKS Cluster issue is not fixed by re-enabling migrations pulumi/pulumi-terraform-bridge#2039

We still hit pulumi/pulumi-terraform-bridge#1667 - the bridge recovers the state using the new schema and puts an explicit nil value for the new property bootstrap_self_managed_addons. This trips up the migration https://github.com/hashicorp/terraform-provider-aws/blob/70abac477ab0ee5d9398151608db6cbbc51bb248/internal/service/eks/cluster_migrate.go#L264-L265 and it runs but does not fill the default true.

VenelinMartinov added a commit that referenced this issue Aug 29, 2024
The eks:Cluster resource has received a new parameter with ForceNew and
a default. When upgrading from an old version before the parameter was
added this triggers a replace.

This PR adds a workaround for that - when the state is read the
`bootstrapSelfManagedAddons` parameter is added with its default value
if not present.

covered by `TestEKSClusterUpgrade`

partially fixes #4410

stacked on #4403
@VenelinMartinov
Copy link
Contributor Author

VenelinMartinov commented Aug 29, 2024

EKS Cluster replacement is fixed but it looks like there's ~10 resources in AWS which might have similar state upgrades: https://github.com/search?q=repo%3Ahashicorp%2Fterraform-provider-aws%20StateUpgraders&type=code

Some of the more notable ones are rds.Instance and rds.Cluster

However most of them are not ForceNew (incl the rds resources) - this meansa diff should be triggered but not a replace.

I did find two with ForceNew: aws_securityhub_account and aws_cloudwatch_event_target which might be affected like EKS Cluster. Neither of them seem stateful, so it might be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants