User/jguionnet/kubecon25 demo#143
Open
jguionnet wants to merge 5 commits into
Open
Conversation
Signed-off-by: jguionnet <jguionnet@guidewire.com>
- Deleted the old setup notebook `00 Setup.ipynb` and replaced it with `00_Env-setup.ipynb` for improved clarity and organization. - Introduced a new cleanup notebook `00-Env-cleanup.ipynb` for environment teardown. - Added `01-OAM-cleanup.ipynb` for cleaning up resources created during the OAM contribution demo. - Updated README to reflect new notebook names and cleanup procedures. Signed-off-by: jguionnet <jguionnet@guidewire.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 8 files
Prompt for AI agents (all 3 issues)
Understand the root cause of the following 3 issues and fix them.
<file name="15.KubeCon_NA_2025_Demo/00-Env-cleanup.ipynb">
<violation number="1" location="15.KubeCon_NA_2025_Demo/00-Env-cleanup.ipynb:170">
The cleanup cells hardcode `CLUSTER_NAME="kubecon-demo"`, so if someone changes `cluster.name` in config.yaml the teardown will skip the actual cluster/context and leave resources running. Please feed the configured name into these bash cells instead of the literal string.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/README.md">
<violation number="1" location="15.KubeCon_NA_2025_Demo/README.md:31">
The README claims a template already exists at `.env.aws`, but that file is gitignored and not present, so users will look for something that isn’t there. Please update the wording to tell readers to create the file (or point to the actual template name).</violation>
<violation number="2" location="15.KubeCon_NA_2025_Demo/README.md:36">
The sample AWS secret key includes a trailing underscore, so anyone copying it ends up with an invalid credential. Please remove the extra underscore so the example reflects a valid key format.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| "%%bash\n", | ||
| "set -e\n", | ||
| "\n", | ||
| "CLUSTER_NAME=\"kubecon-demo\"\n", |
There was a problem hiding this comment.
The cleanup cells hardcode CLUSTER_NAME="kubecon-demo", so if someone changes cluster.name in config.yaml the teardown will skip the actual cluster/context and leave resources running. Please feed the configured name into these bash cells instead of the literal string.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/00-Env-cleanup.ipynb at line 170:
<comment>The cleanup cells hardcode `CLUSTER_NAME="kubecon-demo"`, so if someone changes `cluster.name` in config.yaml the teardown will skip the actual cluster/context and leave resources running. Please feed the configured name into these bash cells instead of the literal string.</comment>
<file context>
@@ -0,0 +1,352 @@
+ "%%bash\n",
+ "set -e\n",
+ "\n",
+ "CLUSTER_NAME=\"kubecon-demo\"\n",
+ "\n",
+ "echo \"=== Deleting k3d Cluster: $CLUSTER_NAME ===\"\n",
</file context>
|
|
||
| ### Step 1: Create `.env.aws` File | ||
|
|
||
| A template has been created at `.env.aws`. Edit it with your credentials: |
There was a problem hiding this comment.
The README claims a template already exists at .env.aws, but that file is gitignored and not present, so users will look for something that isn’t there. Please update the wording to tell readers to create the file (or point to the actual template name).
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/README.md at line 31:
<comment>The README claims a template already exists at `.env.aws`, but that file is gitignored and not present, so users will look for something that isn’t there. Please update the wording to tell readers to create the file (or point to the actual template name).</comment>
<file context>
@@ -0,0 +1,233 @@
+
+### Step 1: Create `.env.aws` File
+
+A template has been created at `.env.aws`. Edit it with your credentials:
+
+```bash
</file context>
Suggested change
| A template has been created at `.env.aws`. Edit it with your credentials: | |
| Create a `.env.aws` file and edit it with your credentials: |
| ```bash | ||
| # .env.aws | ||
| AWS_ACCESS_KEY_ID=your-actual-access-key-id | ||
| AWS_SECRET_ACCESS_KEY=your-actual-secret-access-key_ |
There was a problem hiding this comment.
The sample AWS secret key includes a trailing underscore, so anyone copying it ends up with an invalid credential. Please remove the extra underscore so the example reflects a valid key format.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/README.md at line 36:
<comment>The sample AWS secret key includes a trailing underscore, so anyone copying it ends up with an invalid credential. Please remove the extra underscore so the example reflects a valid key format.</comment>
<file context>
@@ -0,0 +1,233 @@
+```bash
+# .env.aws
+AWS_ACCESS_KEY_ID=your-actual-access-key-id
+AWS_SECRET_ACCESS_KEY=your-actual-secret-access-key_
+AWS_SESSION_TOKEN=your-actual-session-token
+AWS_DEFAULT_REGION=us-west-2
</file context>
Suggested change
| AWS_SECRET_ACCESS_KEY=your-actual-secret-access-key_ | |
| AWS_SECRET_ACCESS_KEY=your-actual-secret-access-key |
- Updated `00_Env-setup.ipynb` and `00-Env-cleanup.ipynb` to remove output cells for cleaner execution. - Modified `01_OAM-contrib.ipynb` and `01-OAM-cleanup.ipynb` to streamline outputs and enhance clarity. - Introduced a new demo plan document `DEMO_PLAN.md` outlining the architecture and scenarios for the KubeVela demo. Signed-off-by: jguionnet <jguionnet@guidewire.com>
- Introduced a new `README.md` in the `KV-demo` directory, outlining the demo's architecture, features, and usage instructions. - Added a Flask-based Product Catalog API with S3 integration, including endpoints for product management and health checks. - Created Dockerfile and scripts for local development and deployment. - Implemented a comprehensive comparison document between traditional and KubeVela approaches for application deployment. Signed-off-by: jguionnet <jguionnet@guidewire.com>
There was a problem hiding this comment.
Reviewed changes from recent commits (found 13 issues).
13 issues found across 24 files
Prompt for AI agents (all 13 issues)
Understand the root cause of the following 13 issues and fix them.
<file name="15.KubeCon_NA_2025_Demo/KV-demo/app/app.py">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/app/app.py:90">
`image_data` is uploaded to S3 using `.encode('utf-8')`, which writes the base64 text rather than decoding it into image bytes. This produces corrupted image objects. Decode the base64 payload (and validate it) before uploading.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/terraform/variables.tf">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/terraform/variables.tf:34">
Empty default OIDC provider ARN makes IAM assume role policy invalid, causing apply failure.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/scripts/setup-aws-credentials.sh">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/scripts/setup-aws-credentials.sh:32">
`kubectl apply` here is fed the raw secret YAML (including resourceVersion/uid) from `kubectl get … -o yaml`, so creating it in another namespace will fail (`resourceVersion` must be empty on create). Strip those metadata fields or recreate the secret without reusing the exported metadata.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/.github/workflows/deploy.yml">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/.github/workflows/deploy.yml:167">
`deploy-staging` depends on `deploy-dev`, but `deploy-dev` is skipped for main/staging/prod runs (its `if` only passes on develop or manual `dev`). In GitHub Actions, a job with `needs` will also be skipped if its dependency is skipped, so staging (and production) never executes. Please remove the dependency or adjust the conditions so staging/prod can run when desired.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/docs/COMPARISON.md">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/docs/COMPARISON.md:131">
Horizontal Pod Autoscaling section states the HPA manifest is 39 lines even though the earlier file breakdown calls it 45 lines. Please align these numbers so the documentation stays self-consistent.</violation>
<violation number="2" location="15.KubeCon_NA_2025_Demo/KV-demo/docs/COMPARISON.md:276">
The Multi-Stage Deployment example references a 210-line GitHub Actions workflow, but the earlier breakdown says the same file has 249 lines. Please pick one accurate number so readers are not misled.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/k8s/serviceaccount.yaml">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/k8s/serviceaccount.yaml:8">
The IRSA annotation hard-codes `ACCOUNT_ID`, so the ServiceAccount ends up pointing at a non-existent IAM role and pods cannot assume AWS credentials.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/01_OAM-contrib.ipynb">
<violation number="1" location="15.KubeCon_NA_2025_Demo/01_OAM-contrib.ipynb:494">
There's an obvious typo in the Step 5 heading (`ApplicationSSSSSS`). Please remove the extra `S` characters to keep the documentation polished.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/README.md">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/README.md:18">
Terraform section understates the number of files (5) and lines (223) in the traditional setup, misrepresenting the comparison.</violation>
<violation number="2" location="15.KubeCon_NA_2025_Demo/KV-demo/README.md:21">
The traditional approach summary misstates the totals (it should be 11 files and ~662 lines), undermining the accuracy of the comparison.</violation>
<violation number="3" location="15.KubeCon_NA_2025_Demo/KV-demo/README.md:83">
The instructions direct readers to change into a `kubevela` directory that does not exist in the repo, so the documented workflow breaks at this step.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/app/test_api.sh">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/app/test_api.sh:4">
Enable pipefail when using `set -e` before `curl | jq` pipelines so connection failures do not get silently ignored.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/terraform/main.tf">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/terraform/main.tf:48">
IRSA trust condition uses the issuer URL with its https scheme, so the federated service account can never assume the role when the cluster’s issuer URL is used as-is, causing all AWS calls to fail.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| s3_client.put_object( | ||
| Bucket=S3_BUCKET, | ||
| Key=image_key, | ||
| Body=data['image_data'].encode('utf-8'), |
There was a problem hiding this comment.
image_data is uploaded to S3 using .encode('utf-8'), which writes the base64 text rather than decoding it into image bytes. This produces corrupted image objects. Decode the base64 payload (and validate it) before uploading.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/app/app.py at line 90:
<comment>`image_data` is uploaded to S3 using `.encode('utf-8')`, which writes the base64 text rather than decoding it into image bytes. This produces corrupted image objects. Decode the base64 payload (and validate it) before uploading.</comment>
<file context>
@@ -0,0 +1,169 @@
+ s3_client.put_object(
+ Bucket=S3_BUCKET,
+ Key=image_key,
+ Body=data['image_data'].encode('utf-8'),
+ ContentType='image/jpeg'
+ )
</file context>
| variable "oidc_provider_arn" { | ||
| description = "ARN of the OIDC provider for EKS/k3d" | ||
| type = string | ||
| default = "" |
There was a problem hiding this comment.
Empty default OIDC provider ARN makes IAM assume role policy invalid, causing apply failure.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/terraform/variables.tf at line 34:
<comment>Empty default OIDC provider ARN makes IAM assume role policy invalid, causing apply failure.</comment>
<file context>
@@ -0,0 +1,56 @@
+variable "oidc_provider_arn" {
+ description = "ARN of the OIDC provider for EKS/k3d"
+ type = string
+ default = ""
+}
+
</file context>
| # Get secret from source namespace and apply to target | ||
| echo " Copying secret ${SECRET_NAME}..." | ||
| if kubectl get secret ${SECRET_NAME} -n ${SOURCE_NAMESPACE} &>/dev/null; then | ||
| kubectl get secret ${SECRET_NAME} -n ${SOURCE_NAMESPACE} -o yaml | \ |
There was a problem hiding this comment.
kubectl apply here is fed the raw secret YAML (including resourceVersion/uid) from kubectl get … -o yaml, so creating it in another namespace will fail (resourceVersion must be empty on create). Strip those metadata fields or recreate the secret without reusing the exported metadata.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/scripts/setup-aws-credentials.sh at line 32:
<comment>`kubectl apply` here is fed the raw secret YAML (including resourceVersion/uid) from `kubectl get … -o yaml`, so creating it in another namespace will fail (`resourceVersion` must be empty on create). Strip those metadata fields or recreate the secret without reusing the exported metadata.</comment>
<file context>
@@ -0,0 +1,62 @@
+ # Get secret from source namespace and apply to target
+ echo " Copying secret ${SECRET_NAME}..."
+ if kubectl get secret ${SECRET_NAME} -n ${SOURCE_NAMESPACE} &>/dev/null; then
+ kubectl get secret ${SECRET_NAME} -n ${SOURCE_NAMESPACE} -o yaml | \
+ sed "s/namespace: ${SOURCE_NAMESPACE}/namespace: ${target_ns}/" | \
+ kubectl apply -f -
</file context>
| deploy-staging: | ||
| name: Deploy to Staging | ||
| runs-on: ubuntu-latest | ||
| needs: deploy-dev |
There was a problem hiding this comment.
deploy-staging depends on deploy-dev, but deploy-dev is skipped for main/staging/prod runs (its if only passes on develop or manual dev). In GitHub Actions, a job with needs will also be skipped if its dependency is skipped, so staging (and production) never executes. Please remove the dependency or adjust the conditions so staging/prod can run when desired.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/.github/workflows/deploy.yml at line 167:
<comment>`deploy-staging` depends on `deploy-dev`, but `deploy-dev` is skipped for main/staging/prod runs (its `if` only passes on develop or manual `dev`). In GitHub Actions, a job with `needs` will also be skipped if its dependency is skipped, so staging (and production) never executes. Please remove the dependency or adjust the conditions so staging/prod can run when desired.</comment>
<file context>
@@ -0,0 +1,249 @@
+ deploy-staging:
+ name: Deploy to Staging
+ runs-on: ubuntu-latest
+ needs: deploy-dev
+ environment: staging
+ if: github.ref == 'refs/heads/main' || github.event.inputs.environment == 'staging'
</file context>
|
|
||
| **Traditional:** | ||
| ```yaml | ||
| # GitHub Actions workflow (210 lines) |
There was a problem hiding this comment.
The Multi-Stage Deployment example references a 210-line GitHub Actions workflow, but the earlier breakdown says the same file has 249 lines. Please pick one accurate number so readers are not misled.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/docs/COMPARISON.md at line 276:
<comment>The Multi-Stage Deployment example references a 210-line GitHub Actions workflow, but the earlier breakdown says the same file has 249 lines. Please pick one accurate number so readers are not misled.</comment>
<file context>
@@ -0,0 +1,518 @@
+
+**Traditional:**
+```yaml
+# GitHub Actions workflow (210 lines)
+jobs:
+ terraform:
</file context>
Suggested change
| # GitHub Actions workflow (210 lines) | |
| # GitHub Actions workflow (249 lines) |
✅ Addressed in 29ac983
| - **Terraform** (4 files, 209 lines) - Infrastructure as Code (one-time) | ||
| - **Kubernetes Manifests** (5 files, 190 lines) - Application deployment (per-app) | ||
| - **GitHub Actions** (1 file, 249 lines) - CI/CD pipeline (per-app) | ||
| - **Total**: 10 files, ~648 lines (209 one-time + 439 per-app) |
There was a problem hiding this comment.
The traditional approach summary misstates the totals (it should be 11 files and ~662 lines), undermining the accuracy of the comparison.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/README.md at line 21:
<comment>The traditional approach summary misstates the totals (it should be 11 files and ~662 lines), undermining the accuracy of the comparison.</comment>
<file context>
@@ -0,0 +1,307 @@
+- **Terraform** (4 files, 209 lines) - Infrastructure as Code (one-time)
+- **Kubernetes Manifests** (5 files, 190 lines) - Application deployment (per-app)
+- **GitHub Actions** (1 file, 249 lines) - CI/CD pipeline (per-app)
+- **Total**: 10 files, ~648 lines (209 one-time + 439 per-app)
+
+#### 2. KubeVela Approach (`/kubevela/`)
</file context>
Suggested change
| - **Total**: 10 files, ~648 lines (209 one-time + 439 per-app) | |
| - **Total**: 11 files, ~662 lines (223 one-time + 439 per-app) |
|
|
||
| #### 1. Traditional Approach (`/comparison/traditional/`) | ||
| The conventional way using multiple tools: | ||
| - **Terraform** (4 files, 209 lines) - Infrastructure as Code (one-time) |
There was a problem hiding this comment.
Terraform section understates the number of files (5) and lines (223) in the traditional setup, misrepresenting the comparison.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/README.md at line 18:
<comment>Terraform section understates the number of files (5) and lines (223) in the traditional setup, misrepresenting the comparison.</comment>
<file context>
@@ -0,0 +1,307 @@
+
+#### 1. Traditional Approach (`/comparison/traditional/`)
+The conventional way using multiple tools:
+- **Terraform** (4 files, 209 lines) - Infrastructure as Code (one-time)
+- **Kubernetes Manifests** (5 files, 190 lines) - Application deployment (per-app)
+- **GitHub Actions** (1 file, 249 lines) - CI/CD pipeline (per-app)
</file context>
Suggested change
| - **Terraform** (4 files, 209 lines) - Infrastructure as Code (one-time) | |
| - **Terraform** (5 files, 223 lines) - Infrastructure as Code (one-time) |
| #### Step 3: Show KubeVela Approach (The Power) | ||
|
|
||
| ```bash | ||
| cd kubevela |
There was a problem hiding this comment.
The instructions direct readers to change into a kubevela directory that does not exist in the repo, so the documented workflow breaks at this step.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/README.md at line 83:
<comment>The instructions direct readers to change into a `kubevela` directory that does not exist in the repo, so the documented workflow breaks at this step.</comment>
<file context>
@@ -0,0 +1,307 @@
+#### Step 3: Show KubeVela Approach (The Power)
+
+```bash
+cd kubevela
+
+# Install Crossplane S3 component (one-time platform setup)
</file context>
| #!/bin/bash | ||
| # Simple test script for the Product Catalog API | ||
|
|
||
| set -e |
There was a problem hiding this comment.
Enable pipefail when using set -e before curl | jq pipelines so connection failures do not get silently ignored.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/app/test_api.sh at line 4:
<comment>Enable pipefail when using `set -e` before `curl | jq` pipelines so connection failures do not get silently ignored.</comment>
<file context>
@@ -0,0 +1,53 @@
+#!/bin/bash
+# Simple test script for the Product Catalog API
+
+set -e
+
+API_URL="${API_URL:-http://localhost:8080}"
</file context>
| Action = "sts:AssumeRoleWithWebIdentity" | ||
| Condition = { | ||
| StringEquals = { | ||
| "${var.oidc_provider_url}:sub" = "system:serviceaccount:${var.namespace}:product-api-sa" |
There was a problem hiding this comment.
IRSA trust condition uses the issuer URL with its https scheme, so the federated service account can never assume the role when the cluster’s issuer URL is used as-is, causing all AWS calls to fail.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/terraform/main.tf at line 48:
<comment>IRSA trust condition uses the issuer URL with its https scheme, so the federated service account can never assume the role when the cluster’s issuer URL is used as-is, causing all AWS calls to fail.</comment>
<file context>
@@ -0,0 +1,95 @@
+ Action = "sts:AssumeRoleWithWebIdentity"
+ Condition = {
+ StringEquals = {
+ "${var.oidc_provider_url}:sub" = "system:serviceaccount:${var.namespace}:product-api-sa"
+ "${var.oidc_provider_url}:aud" = "sts.amazonaws.com"
+ }
</file context>
Suggested change
| "${var.oidc_provider_url}:sub" = "system:serviceaccount:${var.namespace}:product-api-sa" | |
| "${replace(var.oidc_provider_url, "https://", "")}:sub" = "system:serviceaccount:${var.namespace}:product-api-sa" |
- Created a new .gitignore file to exclude Terraform state files, AWS credentials, IDE configurations, and temporary files. - Updated DEMO_PLAN.md to reflect changes in Kubernetes and Terraform resource lines, emphasizing the reduction in complexity with KubeVela. - Revised README.md to clarify the traditional approach and KubeVela comparison, highlighting the benefits of a unified application model. Signed-off-by: jguionnet <jguionnet@guidewire.com>
There was a problem hiding this comment.
Reviewed changes from recent commits (found 9 issues).
9 issues found across 19 files
Prompt for AI agents (all 9 issues)
Understand the root cause of the following 9 issues and fix them.
<file name="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/dagger/README.md">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/dagger/README.md:12">
Running the pipeline from dagger/ causes Dagger to fail to mount ./terraform and ./k8s because those directories live one level up. Please point users to run the command from the traditional/ root (or adjust the code paths) so the quick start succeeds.</violation>
<violation number="2" location="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/dagger/README.md:21">
The pipeline code currently only builds the image and exports it locally; it never pushes to a registry. Please update the description so it matches the actual behavior (or implement the push).</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/terraform/outputs.tf">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/terraform/outputs.tf:23">
When reusing an existing IAM role, splitting the ARN and taking index 1 will return the first path segment (e.g., "service-role") instead of the actual role name if the ARN contains a path. Use the last segment of the ARN instead.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/.gitignore">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/.gitignore:6">
Ignoring `.terraform.lock.hcl` prevents committing the Terraform provider lock file, so different contributors may install different provider versions and the demo can fail or drift. Please allow the lock file to be tracked so everyone uses the same provider revisions.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/docs/COMPARISON.md">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/docs/COMPARISON.md:17">
The Terraform directory is documented as 243 lines, but the actual total is 223; please update the statistic to reflect the real count.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/README.md">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/README.md:21">
Correct the total line counts to 660 (223 one-time + 437 per-app) to match the actual file sizes.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/DEMO_PLAN.md">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/DEMO_PLAN.md:176">
This line promises an application.yaml with 171 lines under KV-demo, but the repository does not contain that file or even a kubevela/ directory. Please update the plan or add the missing artifact so the documentation reflects reality.</violation>
</file>
<file name="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/deploy-local.sh">
<violation number="1" location="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/deploy-local.sh:66">
Cleanup is deleting configmap app-config, but the manifest creates product-api-config, so the ConfigMap survives cleanup. Use the actual name to avoid leftovers.</violation>
<violation number="2" location="15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/deploy-local.sh:147">
The sed replacement never updates the Deployment image tag because it searches for localhost while the manifest uses k3d-registry, so passing a custom IMAGE_TAG keeps deploying the old image. Update the pattern to target the actual registry host so new tags roll out.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| ## What It Does | ||
|
|
||
| 1. Terraform: Creates S3 bucket | ||
| 2. Build: Builds and pushes Docker image |
There was a problem hiding this comment.
The pipeline code currently only builds the image and exports it locally; it never pushes to a registry. Please update the description so it matches the actual behavior (or implement the push).
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/dagger/README.md at line 21:
<comment>The pipeline code currently only builds the image and exports it locally; it never pushes to a registry. Please update the description so it matches the actual behavior (or implement the push).</comment>
<file context>
@@ -0,0 +1,39 @@
+## What It Does
+
+1. Terraform: Creates S3 bucket
+2. Build: Builds and pushes Docker image
+3. Deploy: Applies Kubernetes manifests
+4. Verify: Waits for rollout
</file context>
Suggested change
| 2. Build: Builds and pushes Docker image | |
| 2. Build: Builds the Docker image (exported locally for manual push) |
| curl -L https://dl.dagger.io/dagger/install.sh | sudo sh | ||
|
|
||
| # Run pipeline | ||
| cd dagger |
There was a problem hiding this comment.
Running the pipeline from dagger/ causes Dagger to fail to mount ./terraform and ./k8s because those directories live one level up. Please point users to run the command from the traditional/ root (or adjust the code paths) so the quick start succeeds.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/dagger/README.md at line 12:
<comment>Running the pipeline from dagger/ causes Dagger to fail to mount ./terraform and ./k8s because those directories live one level up. Please point users to run the command from the traditional/ root (or adjust the code paths) so the quick start succeeds.</comment>
<file context>
@@ -0,0 +1,39 @@
+curl -L https://dl.dagger.io/dagger/install.sh | sudo sh
+
+# Run pipeline
+cd dagger
+go mod download
+export ENVIRONMENT=dev IMAGE_TAG=v1.0.0-traditional
</file context>
|
|
||
| output "iam_role_name" { | ||
| description = "Name of the IAM role" | ||
| value = var.create_iam_resources ? aws_iam_role.product_api[0].name : split("/", var.existing_iam_role_arn)[1] |
There was a problem hiding this comment.
When reusing an existing IAM role, splitting the ARN and taking index 1 will return the first path segment (e.g., "service-role") instead of the actual role name if the ARN contains a path. Use the last segment of the ARN instead.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/terraform/outputs.tf at line 23:
<comment>When reusing an existing IAM role, splitting the ARN and taking index 1 will return the first path segment (e.g., "service-role") instead of the actual role name if the ARN contains a path. Use the last segment of the ARN instead.</comment>
<file context>
@@ -14,16 +14,16 @@ output "bucket_region" {
output "iam_role_name" {
description = "Name of the IAM role"
- value = aws_iam_role.product_api.name
+ value = var.create_iam_resources ? aws_iam_role.product_api[0].name : split("/", var.existing_iam_role_arn)[1]
}
</file context>
Suggested change
| value = var.create_iam_resources ? aws_iam_role.product_api[0].name : split("/", var.existing_iam_role_arn)[1] | |
| value = var.create_iam_resources ? aws_iam_role.product_api[0].name : element(split("/", var.existing_iam_role_arn), length(split("/", var.existing_iam_role_arn)) - 1) |
| *.tfstate.* | ||
| *.tfvars.backup | ||
| .terraform/ | ||
| .terraform.lock.hcl |
There was a problem hiding this comment.
Ignoring .terraform.lock.hcl prevents committing the Terraform provider lock file, so different contributors may install different provider versions and the demo can fail or drift. Please allow the lock file to be tracked so everyone uses the same provider revisions.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/.gitignore at line 6:
<comment>Ignoring `.terraform.lock.hcl` prevents committing the Terraform provider lock file, so different contributors may install different provider versions and the demo can fail or drift. Please allow the lock file to be tracked so everyone uses the same provider revisions.</comment>
<file context>
@@ -0,0 +1,34 @@
+*.tfstate.*
+*.tfvars.backup
+.terraform/
+.terraform.lock.hcl
+terraform.tfplan
+tfplan
</file context>
|
|
||
| **Structure:** | ||
| ``` | ||
| terraform/ # 243 lines (one-time) |
There was a problem hiding this comment.
The Terraform directory is documented as 243 lines, but the actual total is 223; please update the statistic to reflect the real count.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/docs/COMPARISON.md at line 17:
<comment>The Terraform directory is documented as 243 lines, but the actual total is 223; please update the statistic to reflect the real count.</comment>
<file context>
@@ -1,518 +1,194 @@
-| **Workflow Definition** | External CI/CD (249 lines) | Built-in | No external CI/CD |
+**Structure:**
+```
+terraform/ # 243 lines (one-time)
+ - S3 bucket: tenant-atlantis-product-images-traditional
+ - IAM: Role ARN configured via ServiceAccount annotation
</file context>
Suggested change
| terraform/ # 243 lines (one-time) | |
| terraform/ # 223 lines (one-time) |
| - **Terraform** (4 files, 243 lines) - Infrastructure as Code (one-time) | ||
| - **Kubernetes Manifests** (5 files, 188 lines) - Application deployment (per-app) | ||
| - **CI/CD Pipeline** (1 file, 249 lines) - GitHub Actions or Dagger (per-app) | ||
| - **Total**: 10 files, 680 lines (243 one-time + 437 per-app) |
There was a problem hiding this comment.
Correct the total line counts to 660 (223 one-time + 437 per-app) to match the actual file sizes.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/README.md at line 21:
<comment>Correct the total line counts to 660 (223 one-time + 437 per-app) to match the actual file sizes.</comment>
<file context>
@@ -15,17 +15,21 @@ A Python Flask + boto3 Product Catalog API that stores product images in S3:
+- **Terraform** (4 files, 243 lines) - Infrastructure as Code (one-time)
+- **Kubernetes Manifests** (5 files, 188 lines) - Application deployment (per-app)
+- **CI/CD Pipeline** (1 file, 249 lines) - GitHub Actions or Dagger (per-app)
+- **Total**: 10 files, 680 lines (243 one-time + 437 per-app)
+
+**Local Execution Options:**
</file context>
| ### Scenario 2: KubeVela (The Better Way) | ||
|
|
||
| **What you need:** | ||
| - application.yaml (171 lines total) |
There was a problem hiding this comment.
This line promises an application.yaml with 171 lines under KV-demo, but the repository does not contain that file or even a kubevela/ directory. Please update the plan or add the missing artifact so the documentation reflects reality.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/DEMO_PLAN.md at line 176:
<comment>This line promises an application.yaml with 171 lines under KV-demo, but the repository does not contain that file or even a kubevela/ directory. Please update the plan or add the missing artifact so the documentation reflects reality.</comment>
<file context>
@@ -175,9 +173,9 @@ A microservice that:
**What you need:**
-- application.yaml (80-100 lines total)
+- application.yaml (171 lines total)
- Component definitions (reusable, platform-provided)
-- **Total: 100 lines in 1 file**
</file context>
Suggested change
| - application.yaml (171 lines total) | |
| - application.yaml (single file once created; update line count when available) |
| kubectl delete hpa product-catalog-api-hpa -n ${ENVIRONMENT} --ignore-not-found=true | ||
| kubectl delete service product-catalog-api -n ${ENVIRONMENT} --ignore-not-found=true | ||
| kubectl delete deployment product-catalog-api -n ${ENVIRONMENT} --ignore-not-found=true | ||
| kubectl delete configmap app-config -n ${ENVIRONMENT} --ignore-not-found=true |
There was a problem hiding this comment.
Cleanup is deleting configmap app-config, but the manifest creates product-api-config, so the ConfigMap survives cleanup. Use the actual name to avoid leftovers.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/deploy-local.sh at line 66:
<comment>Cleanup is deleting configmap app-config, but the manifest creates product-api-config, so the ConfigMap survives cleanup. Use the actual name to avoid leftovers.</comment>
<file context>
@@ -0,0 +1,176 @@
+ kubectl delete hpa product-catalog-api-hpa -n ${ENVIRONMENT} --ignore-not-found=true
+ kubectl delete service product-catalog-api -n ${ENVIRONMENT} --ignore-not-found=true
+ kubectl delete deployment product-catalog-api -n ${ENVIRONMENT} --ignore-not-found=true
+ kubectl delete configmap app-config -n ${ENVIRONMENT} --ignore-not-found=true
+ kubectl delete serviceaccount product-api-sa -n ${ENVIRONMENT} --ignore-not-found=true
+
</file context>
Suggested change
| kubectl delete configmap app-config -n ${ENVIRONMENT} --ignore-not-found=true | |
| kubectl delete configmap product-api-config -n ${ENVIRONMENT} --ignore-not-found=true |
|
|
||
| # Update deployment with correct image tag and namespace | ||
| cat k8s/deployment.yaml | \ | ||
| sed "s|localhost:5000/product-catalog-api:v1.0.0-traditional|localhost:5000/product-catalog-api:${IMAGE_TAG}|g" | \ |
There was a problem hiding this comment.
The sed replacement never updates the Deployment image tag because it searches for localhost while the manifest uses k3d-registry, so passing a custom IMAGE_TAG keeps deploying the old image. Update the pattern to target the actual registry host so new tags roll out.
Prompt for AI agents
Address the following comment on 15.KubeCon_NA_2025_Demo/KV-demo/comparison/traditional/deploy-local.sh at line 147:
<comment>The sed replacement never updates the Deployment image tag because it searches for localhost while the manifest uses k3d-registry, so passing a custom IMAGE_TAG keeps deploying the old image. Update the pattern to target the actual registry host so new tags roll out.</comment>
<file context>
@@ -0,0 +1,176 @@
+
+# Update deployment with correct image tag and namespace
+cat k8s/deployment.yaml | \
+ sed "s|localhost:5000/product-catalog-api:v1.0.0-traditional|localhost:5000/product-catalog-api:${IMAGE_TAG}|g" | \
+ kubectl apply -f - -n ${ENVIRONMENT}
+
</file context>
Suggested change
| sed "s|localhost:5000/product-catalog-api:v1.0.0-traditional|localhost:5000/product-catalog-api:${IMAGE_TAG}|g" | \ | |
| sed "s|k3d-registry.localhost:5000/product-catalog-api:v1.0.0-traditional|k3d-registry.localhost:5000/product-catalog-api:${IMAGE_TAG}|g" | \ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Adds a complete KubeCon NA 2025 demo with notebooks to set up k3d + Crossplane + KubeVela, contribute a simple DynamoDB OAM component, and clean up. Also adds a KubeVela power demo with a Flask Product Catalog app comparing traditional (Terraform + Kubernetes + GitHub Actions) vs KubeVela.
New Features
Migration
Written for commit 29ac983. Summary will update automatically on new commits.