Skip to content

Conversation

@yuhkih
Copy link

@yuhkih yuhkih commented Jun 30, 2025

  1. Fix.

I've got the following error when I run my old terraform script which refers to rhcs module even though I hadn't changed anything.

│ Error: Invalid count argument

│ on .terraform/modules/rosa-hcp/modules/account-iam-resources/main.tf line 71, in resource "random_string" "default_random":
│ 71: count = (var.account_role_prefix != null && var.account_role_prefix != "") ? 0 : 1

│ The "count" value depends on resource attributes that cannot be determined until apply, so Terraform cannot predict
│ how many instances will be created. To work around this, use the -target argument to first apply only the resources
│ that the count depends on.

$

My root module passes a random_string to initialize the "account_role_prefix" variable, but it causes the error in a resource in "account-iam-resources" sub module.

The reason for this error was that [var.account_role_prefix != ""] check added to the resource "random_string.default_random".
random_string is (known after apply) value. (known after apply) can't be compared with "" string at the terraform plan phase.
And I believe this change is also affecting when you specify a random_string for the cluster_name variable in root module because it copies to "account_role_prefix" variable within the rhcs module.

As I checked the "random_string.default_random" attribute, the random value is only referred once at most from the main.tf file with fixed index [0]. There is no reference other than that. So, the count value is not actually used at the moment.

As well as that, the following part itself where the count is generated, which is causing the issue, seems unnecessary after I took a look at the rhcs modules.

count = (var.account_role_prefix != null && var.account_role_prefix != "") ? 0 : 1

I guess whether creating this random_string resource or not doesn't have singinificant meaning in this code.
At the moment, I thought the count in random_string.default_random would be better to be removed to prevent people from confusion in the future and make it simpler. So, I removed the related part.

  1. update deprecated aws attribute

I've got the error below so I replaced old aws attribute with the latest one.

│ Warning: Deprecated attribute

│ on .terraform/modules/rosa-hcp/modules/oidc-config-and-provider/main.tf line 69, in resource "rhcs_rosa_oidc_config_input" "oidc_input":
│ 69: region = data.aws_region.current.name

│ The attribute "name" is deprecated. Refer to the provider documentation for details.

@openshift-ci openshift-ci bot requested review from nirarg and robpblake June 30, 2025 02:14
@openshift-ci
Copy link

openshift-ci bot commented Jun 30, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yuhkih
Once this PR has been reviewed and has the lgtm label, please assign ciaranroche for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link

openshift-ci bot commented Jun 30, 2025

Hi @yuhkih. Thanks for your PR.

I'm waiting for a terraform-redhat member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@hunterkepley
Copy link
Member

/ok-to-test

@hunterkepley
Copy link
Member

Hi @yuhkih - thanks for this merge request

One thing we must do, to appease the pipeline, is change the title of the MR, and the commit message, to a format such as this:

TICKET-#### | type: Description

For example, if you do not have a GitHub issue, nor ticket available to supply in place, this would suffice:

ISSUE-1234 | fix: "account-iam-resources" submodule deprecated AWS provider attribute

I will wait on the tests, then review the MR, in the meantime 🙂

@yuhkih yuhkih changed the title A fix for "account-iam-resources" sub module" and a small update to replace deprecated aws provider attribute with the latest one. ISSUE-1234 | fix: "account-iam-resources" sub module and update on deprecated aws provider attribute. Oct 31, 2025
@yuhkih
Copy link
Author

yuhkih commented Oct 31, 2025

Hi, @hunterkepley . Thank you for the comment.
I changed the title and commit message. The header "ISSUE-1234" is a dummy because I don't have any ticket. So, if it's not needed, please let me know.

And as I went through the existing issues. It seems my PR fixes the following issue.
#87

@openshift-ci
Copy link

openshift-ci bot commented Oct 31, 2025

@yuhkih: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/rosa-hcp-public d5f4705 link true /test rosa-hcp-public
ci/prow/verify-gen d5f4705 link true /test verify-gen
ci/prow/rosa-hcp-private d5f4705 link true /test rosa-hcp-private
ci/prow/images d5f4705 link true /test images
ci/prow/verify-format d5f4705 link true /test verify-format

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants