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

Add extra_args_array Field #1212

Closed
wants to merge 1 commit into from
Closed

Add extra_args_array Field #1212

wants to merge 1 commit into from

Conversation

khrisrichardson
Copy link

@khrisrichardson khrisrichardson commented Aug 31, 2023

Issue:

#1209

Problem

Two new fields were added to RKE, ExtraArgsArray and WindowsExtraArgsArray. These fields allow users to specify an extraArg multiple times with different values. The provider now needs to be updated to support these two new fields.

Solution

An extra_args_array set field was added adjacent to the existing extra_args field.

Testing

Engineering Testing

Manual Testing

Here's a snippet demonstrating which fields were added, with everything else in ellipsis.

resource "rancher2_cluster" "cluster" {

  rke_config {
    services {
      kube_api {
        extra_args_array {
          name  = "api-audiences"
          value = [
            "sts.amazonaws.com",
          ]
        }
        extra_args_array {
          name  = "service-account-key-file"
          value = [
            "/etc/kubernetes/ssl/sa-signer-pkcs8.pub",
          ]
        }
        extra_args_array {
          name  = "service-account-issuer"
          value = [
            "rke",
          ]
        }
      }
   }
}

And these were the results, trimmed for clarity

      ~ rke_config {
          ~ services {
              ~ kube_api {
                  + extra_args_array {
                      + name  = "api-audiences"
                      + value = [
                          + "sts.amazonaws.com",
                        ]
                    }
                  + extra_args_array {
                      + name  = "service-account-issuer"
                      + value = [
                          + "rke",
                        ]
                    }
                  + extra_args_array {
                      + name  = "service-account-key-file"
                      + value = [
                          + "/etc/kubernetes/ssl/sa-signer-pkcs8.pub",
                        ]
                    }
                }
            }
        }
    }

Automated Testing

Tests have been added only to rancher2/structure_cluster_rke_config_services_etcd_test.go, since the tests for the other services would otherwise use identical schemas, flatteners, and expanders.

QA Testing Considerations

Regressions Considerations

@khrisrichardson khrisrichardson marked this pull request as ready for review November 11, 2023 23:59
@khrisrichardson khrisrichardson changed the title Add extra_args_array Field #1209 Add extra_args_array Field Nov 12, 2023
@krichardson-tubi
Copy link

CC: @a-blender, could you recommend a reviewer for this PR please? My apologies for keeping it in draft for so long.

@khrisrichardson
Copy link
Author

CC: @snasovich, would you by any chance be able to assign a reviewer? Thanks

Copy link

@richardcase richardcase left a comment

Choose a reason for hiding this comment

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

Thanks @khrisrichardson .

Is it ok to specify both extra_args and the new extra_args_array?

@khrisrichardson
Copy link
Author

Is it ok to specify both extra_args and the new extra_args_array?

@richardcase that is correct, just as they are not mutually exclusive in the rancher/rancher or rancher/rke code

@krichardson-tubi
Copy link

@richardcase do you have any additional concerns or questions?

@khrisrichardson khrisrichardson closed this by deleting the head repository Jun 23, 2024
@nobbs
Copy link
Contributor

nobbs commented Nov 8, 2024

Any chance to get this PR reopened and merged? We're facing an issue as we need to update the service-account-issuer argument for the kube apiserver of a rke-based cluster without breaking every service account token based authentication. Kube-Apiserver does support specifying this argument multiple times, but without support in the terraform provider, we can't make use of it.

@khrisrichardson
Copy link
Author

@nobbs you made the right move opening a new PR. Godspeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants