Revert "[Bugfix] Applying K8s manifests to GKE clusters via URL"

Author: shubpal07

modules/management/kubectl-apply/README.md

@@ -13,8 +13,6 @@ This module simplifies the following functionality:
   * **Raw String:** Specify manifests directly within the module configuration using the `content: manifest_body` format.
   * **File/Template/Directory Reference:** Set `source` to the path to:
     * A single URL to a manifest file. Ex.: `https://github.com/.../myrepo/manifest.yaml`.
-
-    > **Note:** Applying from a URL has important limitations. Please review the [Considerations & Callouts for Applying from URLs](#applying-manifests-from-urls-considerations--callouts) section below.
     * A single local YAML manifest file (`.yaml`). Ex.: `./manifest.yaml`.
     * A template file (`.tftpl`) to generate a manifest. Ex.: `./template.yaml.tftpl`. You can pass the variables to format the template file in `template_vars`.
     * A directory containing multiple YAML or template files. Ex: `./manifests/`. You can pass the variables to format the template files in `template_vars`.
@@ -97,73 +95,6 @@ You can specify a particular kueue version that you would like to use using the
 >
 > Terraform may apply resources in parallel, leading to potential dependency issues. If a resource's dependencies aren't ready, it will be applied again up to 15 times.
 
-## Callouts
-
-### Applying Manifests from URLs: Considerations & Callouts
-
-While this module supports applying manifests directly from remote `http://` or `https://` URLs, this method introduces complexities not present when using local files. For production environments, we recommend sourcing manifests from local paths or a version-controlled Git repository. Moreover, this method will be deprecated soon. Hence we recommend to use other methods to source manifests.
-
-If you choose to use the URL method, be aware of the following potential issues and their solutions.
-
-#### **1. Apply Order and Race Conditions**
-
-The module applies manifests from the `apply_manifests` list in parallel. This can create a **race condition** if one manifest depends on another. The most common example is applying a manifest with custom resources (like a `ClusterQueue`) at the same time as the manifest that defines it (the `CustomResourceDefinition` or CRD).
-
-There is **no guarantee** that the CRD will be applied before the resource that uses it. This can lead to non-deterministic deployment failures with errors like:
-
-```Error: resource [kueue.x-k8s.io/v1beta1/ClusterQueue] isn't valid for cluster```
-
-##### **Recommended Workaround: Two-Stage Apply**
-
-To ensure a reliable deployment, you must manually enforce the correct order of operations.
-
-1. **Initial Deployment:** In your blueprint, include **only** the manifest(s) containing the `CustomResourceDefinition` (CRD) resources in the `apply_manifests` list.
-
-    *Example `settings` for the first run:*
-
-    ```yaml
-    settings:
-      apply_manifests:
-      # This manifest contains the CRDs for Kueue
-      - source: "https://raw.githubusercontent.com/GoogleCloudPlatform/cluster-toolkit/refs/heads/develop/modules/management/kubectl-apply/manifests/kueue-v0.11.4.yaml"
-        server_side_apply: true
-    ```
-
-2. **Run the deployment** (`gcluster deploy` or `terraform apply`).
-
-3. **Second Deployment:** Once the first apply is successful, **add** the manifests containing your custom resources (like `ClusterQueue`, `LocalQueue`) to the list.
-
-    *Example `settings` for the second run:*
-
-    ```yaml
-    settings:
-      apply_manifests:
-      # The CRD manifest is still present
-      - source: "https://raw.githubusercontent.com/GoogleCloudPlatform/cluster-toolkit/refs/heads/develop/modules/management/kubectl-apply/manifests/kueue-v0.11.4.yaml"
-        server_side_apply: true
-
-      # Now, add your configuration manifest
-      - source: "https://gist.githubusercontent.com/YourUser/..." # Your configuration URL
-        server_side_apply: true
-    ```
-
-4. **Run the deployment command again.** Since the CRDs are now guaranteed to exist in the cluster, this second apply will succeed reliably.
-
-#### **2. Large Manifests (CRDs)**
-
-* **Issue:** Applying very large manifests can fail with a `metadata.annotations: Too long` error.
-* **Solution:** Enable Server-Side Apply by setting `server_side_apply: true` for the manifest entry.
-
-#### **3. Conflicts on Re-application**
-
-* **Issue:** Re-running a deployment after a partial failure can cause server-side apply field manager `conflicts`.
-* **Solution:** Forcibly take ownership of the resource fields by setting `force_conflicts: true`.
-
-#### **4. Terraform Template Files (`.tftpl`)**
-
-* **Limitation:** This module **cannot** render a template file (`.tftpl`) when sourced from a remote URL.
-* **Workaround:** You must render the template into a pure YAML file locally, host that rendered file at a URL, and provide the URL of the rendered file in your blueprint.
-
 ## License
 
 <!-- BEGINNING OF PRE-COMMIT-TERRAFORM DOCS HOOK -->
@@ -188,15 +119,13 @@ limitations under the License.
 | <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.3 |
 | <a name="requirement_google"></a> [google](#requirement\_google) | > 5.0 |
 | <a name="requirement_helm"></a> [helm](#requirement\_helm) | ~> 2.17 |
-| <a name="requirement_http"></a> [http](#requirement\_http) | ~> 3.0 |
 | <a name="requirement_kubectl"></a> [kubectl](#requirement\_kubectl) | >= 1.7.0 |
 
 ## Providers
 
 | Name | Version |
 |------|---------|
 | <a name="provider_google"></a> [google](#provider\_google) | > 5.0 |
-| <a name="provider_http"></a> [http](#provider\_http) | ~> 3.0 |
 | <a name="provider_terraform"></a> [terraform](#provider\_terraform) | n/a |
 
 ## Modules
@@ -221,13 +150,12 @@ limitations under the License.
 | [terraform_data.kueue_validations](https://registry.terraform.io/providers/hashicorp/terraform/latest/docs/resources/data) | resource |
 | [google_client_config.default](https://registry.terraform.io/providers/hashicorp/google/latest/docs/data-sources/client_config) | data source |
 | [google_container_cluster.gke_cluster](https://registry.terraform.io/providers/hashicorp/google/latest/docs/data-sources/container_cluster) | data source |
-| [http_http.manifest_from_url](https://registry.terraform.io/providers/hashicorp/http/latest/docs/data-sources/http) | data source |
 
 ## Inputs
 
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
-| <a name="input_apply_manifests"></a> [apply\_manifests](#input\_apply\_manifests) | A list of manifests to apply to GKE cluster using kubectl. For more details see [kubectl module's inputs](kubectl/README.md).<br/> NOTE: The `enable` input acts as a FF to apply a manifest or not. By default it is always set to `true`. | <pre>list(object({<br/>    enable            = optional(bool, true)<br/>    content           = optional(string, null)<br/>    source            = optional(string, null)<br/>    template_vars     = optional(map(any), null)<br/>    server_side_apply = optional(bool, false)<br/>    wait_for_rollout  = optional(bool, true)<br/>  }))</pre> | `[]` | no |
+| <a name="input_apply_manifests"></a> [apply\_manifests](#input\_apply\_manifests) | A list of manifests to apply to GKE cluster using kubectl. For more details see [kubectl module's inputs](kubectl/README.md). | <pre>list(object({<br/>    enable            = optional(bool, true)<br/>    content           = optional(string, null)<br/>    source            = optional(string, null)<br/>    template_vars     = optional(map(any), null)<br/>    server_side_apply = optional(bool, false)<br/>    wait_for_rollout  = optional(bool, true)<br/>  }))</pre> | `[]` | no |
 | <a name="input_cluster_id"></a> [cluster\_id](#input\_cluster\_id) | An identifier for the gke cluster resource with format projects/<project\_id>/locations/<region>/clusters/<name>. | `string` | n/a | yes |
 | <a name="input_gib"></a> [gib](#input\_gib) | Install the NCCL gIB plugin | <pre>object({<br/>    install = bool<br/>    path    = string<br/>    template_vars = object({<br/>      image   = optional(string, "us-docker.pkg.dev/gce-ai-infra/gpudirect-gib/nccl-plugin-gib")<br/>      version = string<br/>      node_affinity = optional(any, {<br/>        requiredDuringSchedulingIgnoredDuringExecution = {<br/>          nodeSelectorTerms = [{<br/>            matchExpressions = [{<br/>              key      = "cloud.google.com/gke-gpu",<br/>              operator = "In",<br/>              values   = ["true"]<br/>            }]<br/>          }]<br/>        }<br/>      })<br/>      accelerator_count = number<br/>    })<br/>  })</pre> | <pre>{<br/>  "install": false,<br/>  "path": "",<br/>  "template_vars": {<br/>    "accelerator_count": 0,<br/>    "version": ""<br/>  }<br/>}</pre> | no |
 | <a name="input_gke_cluster_exists"></a> [gke\_cluster\_exists](#input\_gke\_cluster\_exists) | A static flag that signals to downstream modules that a cluster has been created. Needed by community/modules/scripts/kubernetes-operations. | `bool` | `false` | no |

modules/management/kubectl-apply/kubectl/README.md

@@ -18,12 +18,14 @@ limitations under the License.
 | Name | Version |
 |------|---------|
 | <a name="requirement_terraform"></a> [terraform](#requirement\_terraform) | >= 1.3 |
+| <a name="requirement_http"></a> [http](#requirement\_http) | ~> 3.0 |
 | <a name="requirement_kubectl"></a> [kubectl](#requirement\_kubectl) | >= 1.7.0 |
 
 ## Providers
 
 | Name | Version |
 |------|---------|
+| <a name="provider_http"></a> [http](#provider\_http) | ~> 3.0 |
 | <a name="provider_kubectl"></a> [kubectl](#provider\_kubectl) | >= 1.7.0 |
 
 ## Modules
@@ -35,6 +37,7 @@ No modules.
 | Name | Type |
 |------|------|
 | [kubectl_manifest.apply_doc](https://registry.terraform.io/providers/gavinbunney/kubectl/latest/docs/resources/manifest) | resource |
+| [http_http.yaml_content](https://registry.terraform.io/providers/hashicorp/http/latest/docs/data-sources/http) | data source |
 | [kubectl_path_documents.templates](https://registry.terraform.io/providers/gavinbunney/kubectl/latest/docs/data-sources/path_documents) | data source |
 | [kubectl_path_documents.yamls](https://registry.terraform.io/providers/gavinbunney/kubectl/latest/docs/data-sources/path_documents) | data source |
 

modules/management/kubectl-apply/kubectl/main.tf

@@ -17,44 +17,35 @@
 locals {
   yaml_separator = "\n---"
 
-  # This locals block processes manifest inputs from one of four methods,
-  # evaluated in order of precedence using coalesce.
-
-  # --- METHOD 1: Direct Content Input ---
-  # Used when manifest content is passed directly as a string.
   content_yaml_body = var.content
 
-  # Fallback for safe path checking in subsequent methods.
   null_safe_source = coalesce(var.source_path, " ")
 
-  # --- METHOD 2: Single Local YAML File ---
-  # Used when var.source_path points to a local .yaml file.
-  yaml_file         = length(regexall("\\.yaml(_.*)?$", lower(local.null_safe_source))) == 1 ? abspath(var.source_path) : null
+  url         = startswith(local.null_safe_source, "http://") || startswith(local.null_safe_source, "https://") ? var.source_path : null
+  url_content = local.url != null ? data.http.yaml_content[0].response_body : null
+
+  yaml_file         = local.url == null && length(regexall("\\.yaml(_.*)?$", lower(local.null_safe_source))) == 1 ? abspath(var.source_path) : null
   yaml_file_content = local.yaml_file != null ? file(local.yaml_file) : null
 
-  # --- METHOD 3: Single Local Template File ---
-  # Used when var.source_path points to a local .tftpl file.
-  template_file         = length(regexall("\\.tftpl(_.*)?$", lower(local.null_safe_source))) == 1 ? abspath(var.source_path) : null
+  template_file         = local.url == null && length(regexall("\\.tftpl(_.*)?$", lower(local.null_safe_source))) == 1 ? abspath(var.source_path) : null
   template_file_content = local.template_file != null ? templatefile(local.template_file, var.template_vars) : null
 
-  # --- CONSOLIDATE & PROCESS ---
-  # Coalesce finds the first non-null content from the methods above.
-  yaml_body      = coalesce(local.content_yaml_body, local.yaml_file_content, local.template_file_content, " ")
-  yaml_body_docs = [for doc in split(local.yaml_separator, local.yaml_body) : trimspace(doc) if length(trimspace(doc)) > 0]
+  yaml_body      = coalesce(local.content_yaml_body, local.url_content, local.yaml_file_content, local.template_file_content, " ")
+  yaml_body_docs = [for doc in split(local.yaml_separator, local.yaml_body) : trimspace(doc) if length(trimspace(doc)) > 0] # Split yaml to single docs because the kubectl provider only supports single resource
 
-  # --- METHOD 4: Directory of Files ---
-  # If no content was found via the methods above AND the source path looks like a directory,
-  # we assume this is the desired method. The data blocks below will handle it.
   directory = length(local.yaml_body_docs) == 0 && endswith(local.null_safe_source, "/") ? abspath(var.source_path) : null
 
-  # --- FINAL AGGREGATION ---
-  # Combine documents from single-source methods and directory-scan methods into one list.
   docs_list = concat(try(local.yaml_body_docs, []), try(data.kubectl_path_documents.yamls[0].documents, []), try(data.kubectl_path_documents.templates[0].documents, []))
   docs_map = tomap({
     for index, doc in local.docs_list : index => doc
   })
 }
 
+data "http" "yaml_content" {
+  count = local.url != null ? 1 : 0
+  url   = local.url
+}
+
 data "kubectl_path_documents" "yamls" {
   count   = local.directory != null ? 1 : 0
   pattern = "${local.directory}/*.yaml"

modules/management/kubectl-apply/kubectl/versions.tf

@@ -20,6 +20,10 @@ terraform {
       source  = "gavinbunney/kubectl"
       version = ">= 1.7.0"
     }
+    http = {
+      source  = "hashicorp/http"
+      version = "~> 3.0"
+    }
   }
 
   required_version = ">= 1.3"

modules/management/kubectl-apply/main.tf

@@ -20,34 +20,9 @@ locals {
   cluster_location = local.cluster_id_parts[3]
   project_id       = var.project_id != null ? var.project_id : local.cluster_id_parts[1]
 
-  # 1. First, Identify manifests that are explicitly enabled.
-  enabled_manifests = {
+  apply_manifests_map = tomap({
     for index, manifest in var.apply_manifests : index => manifest
-    if try(manifest.enable, true)
-  }
-
-  # 2. Identify URL-based manifests
-  url_manifests = {
-    for index, manifest in local.enabled_manifests : index => manifest
-    if try(manifest.source, "") != "" && (
-      startswith(manifest.source, "http://") || startswith(manifest.source, "https://")
-    )
-  }
-
-  # 3. Rebuild the map by populating the 'content' field for URLs based manifest
-  processed_apply_manifests_map = tomap({
-    for index, manifest in local.enabled_manifests : tostring(index) => {
-      # If this manifest was a URL, its content is the body from the HTTP call.
-      content = contains(keys(local.url_manifests), tostring(index)) ? data.http.manifest_from_url[tostring(index)].body : manifest.content
-
-      # If this was a URL, its source path is now null. Otherwise, use original.
-      source = contains(keys(local.url_manifests), tostring(index)) ? null : manifest.source
-
-      # Pass other vars
-      template_vars     = manifest.template_vars
-      server_side_apply = manifest.server_side_apply
-      wait_for_rollout  = manifest.wait_for_rollout
-    }
+    if manifest.enable
   })
 
   install_kueue             = try(var.kueue.install, false)
@@ -59,11 +34,6 @@ locals {
   jobset_install_source     = format("${path.module}/manifests/jobset-%s.yaml", try(var.jobset.version, ""))
 }
 
-data "http" "manifest_from_url" {
-  for_each = local.url_manifests
-  url      = each.value.source
-}
-
 data "google_container_cluster" "gke_cluster" {
   project  = local.project_id
   name     = local.cluster_name
@@ -73,7 +43,7 @@ data "google_container_cluster" "gke_cluster" {
 data "google_client_config" "default" {}
 
 module "kubectl_apply_manifests" {
-  for_each   = local.processed_apply_manifests_map
+  for_each   = local.apply_manifests_map
   source     = "./kubectl"
   depends_on = [var.gke_cluster_exists]
 

modules/management/kubectl-apply/variables.tf

@@ -75,7 +75,7 @@ variable "cluster_id" {
 }
 
 variable "apply_manifests" {
-  description = "A list of manifests to apply to GKE cluster using kubectl. For more details see [kubectl module's inputs](kubectl/README.md).\n NOTE: The `enable` input acts as a FF to apply a manifest or not. By default it is always set to `true`. "
+  description = "A list of manifests to apply to GKE cluster using kubectl. For more details see [kubectl module's inputs](kubectl/README.md)."
   type = list(object({
     enable            = optional(bool, true)
     content           = optional(string, null)

modules/management/kubectl-apply/versions.tf

@@ -28,10 +28,6 @@ terraform {
       source  = "hashicorp/helm"
       version = "~> 2.17"
     }
-    http = {
-      source  = "hashicorp/http"
-      version = "~> 3.0"
-    }
   }
 
   provider_meta "google" {