Merge pull request GoogleCloudPlatform#4352 from shubpal07/shubham/bugs/apply-manifest-from-url-v2

[v2][Bugfix] Applying K8s manifests to GKE clusters via URL

Author: shubpal07

modules/management/kubectl-apply/README.md

@@ -13,6 +13,8 @@ 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`.
@@ -95,6 +97,73 @@ 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 -->
@@ -119,13 +188,15 @@ 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
@@ -150,12 +221,13 @@ 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). | <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).<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_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,14 +18,12 @@ 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
@@ -37,7 +35,6 @@ 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,35 +17,44 @@
 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, " ")
 
-  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
+  # --- 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
   yaml_file_content = local.yaml_file != null ? file(local.yaml_file) : null
 
-  template_file         = local.url == null && length(regexall("\\.tftpl(_.*)?$", lower(local.null_safe_source))) == 1 ? abspath(var.source_path) : 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_content = local.template_file != null ? templatefile(local.template_file, var.template_vars) : null
 
-  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
+  # --- 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]
 
+  # --- 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,10 +20,6 @@ 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,9 +20,32 @@ locals {
   cluster_location = local.cluster_id_parts[3]
   project_id       = var.project_id != null ? var.project_id : local.cluster_id_parts[1]
 
-  apply_manifests_map = tomap({
+  # 1. First, Identify manifests that are explicitly enabled.
+  enabled_manifests = {
     for index, manifest in var.apply_manifests : index => manifest
-    if manifest.enable
+    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, null) != null && (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
+    }
   })
 
   install_kueue             = try(var.kueue.install, false)
@@ -34,6 +57,11 @@ 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
@@ -43,7 +71,7 @@ data "google_container_cluster" "gke_cluster" {
 data "google_client_config" "default" {}
 
 module "kubectl_apply_manifests" {
-  for_each   = local.apply_manifests_map
+  for_each   = local.processed_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)."
+  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`. "
   type = list(object({
     enable            = optional(bool, true)
     content           = optional(string, null)

modules/management/kubectl-apply/versions.tf

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