From 95afbf13d9002beda369ac88235a61bfe9ff155c Mon Sep 17 00:00:00 2001
From: Fred Heinecke <fred.heinecke@goteleport.com>
Date: Tue, 6 Feb 2024 15:54:40 -0600
Subject: [PATCH] Addressed PR comments

---
 .github/workflows/tag-build.yaml        | 57 +++++++++++++------------
 .github/workflows/tag-publish.yaml      | 27 ++++++------
 tooling/bin/tf-promote-local.sh         |  6 +--
 tooling/cmd/promote-terraform/README.md |  2 +-
 4 files changed, 45 insertions(+), 47 deletions(-)

diff --git a/.github/workflows/tag-build.yaml b/.github/workflows/tag-build.yaml
index fde9f1eac..add94a053 100644
--- a/.github/workflows/tag-build.yaml
+++ b/.github/workflows/tag-build.yaml
@@ -22,7 +22,7 @@ on:
       - master
 
 concurrency:
-  group: "Limit to one build at a time for ref ${{ format('refs/tags/{0}', inputs.artifact-tag) || github.head_ref }}"
+  group: "Limit to one build at a time for ref ${{ inputs.artifact-tag || github.head_ref || github.ref }}"
   cancel-in-progress: true
 
 jobs:
@@ -38,12 +38,10 @@ jobs:
       - name: Determine git ref
         id: set-gitref
         env:
-          EVENT_NAME: ${{ github.event_name}}
-          REF_TYPE: ${{ github.ref_type }}
-          REF_VALUE: ${{ inputs.artifact-tag || github.head_ref }}
+          REF_VALUE: ${{ inputs.artifact-tag || github.head_ref || github.ref }}
         run: |
           # If value is a tag
-          if [ "$EVENT_NAME" == "workflow_dispatch" ] || [ "$REF_TYPE" == "tag" ]; then
+          if [ "$GITHUB_EVENT_NAME" == "workflow_dispatch" ] || [ "$GITHUB_REF_TYPE" == "tag" ]; then
             echo "gitref=refs/tags/$REF_VALUE" >> "$GITHUB_OUTPUT"
             exit 0
           fi
@@ -58,12 +56,12 @@ jobs:
       - name: Set environment output values
         id: set-variables
         env:
-          GITHUB_EVENT_NAME: ${{ github.event_name}}
           INPUT_VERSION: ${{ inputs.artifact-tag }}
           SEMVER_REGEX: ^v?(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)(?:-(?:(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+(?:[0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$
         run: |
           generate_version() {
               # Example: v1.2.3-gen.4+g5678abcd
+              # If HEAD is tagged (and matches the format) then the output will be just the tag (no commit count or hash)
               git describe --tags --match "v[[:digit:]]*.[[:digit:]]*.[[:digit:]]" | sed 's/\(.*\)-\(.*\)-\(.*\)/\1-gen.\2+\3/'
           }
           
@@ -76,28 +74,28 @@ jobs:
                       ;;
                   "pull_request")
                       echo "environment=build-stage"
-                      echo "version=$(generate_version "$GITHUB_EVENT_REF_NAME")"
+                      echo "version=$(generate_version)"
                       ;;
                   "push")
                       REF_TYPE=$(echo "$GITHUB_EVENT_REF" | cut -d'/' -f2)
                       # Case: commit push event.
                       if [ "$REF_TYPE" != "tags" ]; then
                           echo "environment=build-stage"
-                          echo "version=$(generate_version "$GITHUB_EVENT_REF_NAME")"
+                          echo "version=$(generate_version)"
                           return
                       fi
           
                       # Case: tag event with prerelease version.
-                      if [ "${GITHUB_EVENT_REF_NAME#*-}" != "$GITHUB_EVENT_REF_NAME" ]; then
+                      if [ "${GITHUB_REF_NAME#*-}" != "$GITHUB_REF_NAME" ]; then
                           echo "environment=build-stage"
-                          echo "version=$GITHUB_EVENT_REF_NAME"
+                          echo "version=$GITHUB_REF_NAME"
                           return
                       fi
           
                       # Case: tag event with release version. Only this
                       # should go to prod.
                       echo "environment=build-prod"
-                      echo "version=$GITHUB_EVENT_REF_NAME"
+                      echo "version=$GITHUB_REF_NAME"
                       ;;
                   *)
                       >&2 echo "Unknown GHA event $GITHUB_EVENT_NAME, failing" 
@@ -110,13 +108,11 @@ jobs:
           
           # Validate the semver
           . "$GITHUB_OUTPUT"  # Load the variables into the current environment
-          (echo "$version" | grep -qP "$SEMVER_REGEX") || (echo "The artifact version $version is not a valid semver-coerced value"; exit 1)
+          echo "$version" | grep -qP "$SEMVER_REGEX" || { echo "The artifact version $version is not a valid semver-coerced value"; exit 1; }
         
           # Log the build details
           echo "Built config:" | tee -a "$GITHUB_STEP_SUMMARY"
-          cat "$GITHUB_OUTPUT" | while read line; do
-            echo "* $line" | tee -a "$GITHUB_STEP_SUMMARY"
-          done
+          sed 's/^/* /' "$GITHUB_OUTPUT" | tee -a "$GITHUB_STEP_SUMMARY"
       - name: ${{ inputs.workflow-tag }}
         if: inputs.workflow-tag != ''
         run: |
@@ -125,8 +121,7 @@ jobs:
   # This would improve the (already somewhat fast) performance a bit, but I'm not sure if it's worth the
   # tradeoff.
   build-plugins:
-    needs:
-      - setup
+    needs: setup
     runs-on: ubuntu-22.04-32core
     environment: ${{ needs.setup.outputs.environment }}
     permissions:
@@ -149,7 +144,6 @@ jobs:
         uses: actions/setup-go@v5
         with:
           go-version-file: "./go.mod"
-          cache-dependency-path: "./go.sum"
           check-latest: true
       - name: Set environment variables for Makefiles
         env:
@@ -157,14 +151,20 @@ jobs:
         run: |
           {
             echo "VERSION=${VERSION_TAG##v}"
-            echo "GITREF=$VERSION_TAG" >> "$GITHUB_ENV"
+            echo "GITREF=$VERSION_TAG"
             echo "GNUMAKEFLAGS=-j$(nproc)"
           } >> "$GITHUB_ENV"
       # File artifacts
       - name: Build the release tarballs
         run: |
-          # Binaries and Helm charts
-          make releases helm-package-charts
+          # Download Go dependencies
+          go mod download
+
+          # Build Binaries
+          make releases 
+          
+          # Build Helm charts
+          make helm-package-charts
 
           # Terraform provider and event handler, as appropriate
           go install github.com/konoui/lipo@latest  # At some point this should be merged into the buildbox
@@ -176,11 +176,14 @@ jobs:
       - name: Collect the build files
         run: |
           mkdir -pv "$ARTIFACT_DIRECTORY"
-          cp $(find . '(' -name "*.tar.gz" -o -name "*.tgz" ')' -type f) "$ARTIFACT_DIRECTORY/"
+          find . \( -name '*.tar.gz' -o -name '*.tgz' \) -type f -exec cp {} "$ARTIFACT_DIRECTORY" \;
       - name: Generate checksum files for built files
         working-directory: ${{ env.ARTIFACT_DIRECTORY }}
         run: |
-          find . '(' -name "*.tar.gz" -o -name "*.tgz" ')' -type f -exec sh -c 'sha256sum "$(basename {})" > "{}.sha256"' \;
+          shopt -s nullglob
+          for tarball in *.tar.gz *.tgz; do
+            sha256sum "$(basename "$tarball")" > "${tarball}.sha256"
+          done
           echo "Artifacts:"
           ls -lh
       - name: Assume AWS role for uploading the artifacts
@@ -189,14 +192,14 @@ jobs:
           role-skip-session-tagging: true
           aws-region: us-west-2
           role-to-assume: ${{ vars.ARTIFACT_UPLOAD_AWS_ROLE }}
-          role-session-name: "artifact-upload-${{ github.run_number }}"
+          role-session-name: "tag-build-artifact-upload-${{ github.run_attempt }}"
           role-duration-seconds: 900
       - name: Upload artifacts to S3
         working-directory: ${{ env.ARTIFACT_DIRECTORY }}
         env:
-          ARTIFACT_BUCKET: ${{ vars.ARTIFACT_SOURCE_BUCKET }}
+          PENDING_BUCKET: ${{ vars.PENDING_BUCKET }}
           ARTIFACT_VERSION: ${{ needs.setup.outputs.version }}
-        run: aws s3 cp . "s3://$ARTIFACT_BUCKET/teleport-plugins/tag/$ARTIFACT_VERSION/" --recursive
+        run: aws s3 cp . "s3://$PENDING_BUCKET/teleport-plugins/tag/$ARTIFACT_VERSION/" --recursive
       # Container artifacts
       - name: Assume AWS role for pushing the container images
         uses: aws-actions/configure-aws-credentials@010d0da01d0b5a38af31e9c3470dbfdabdecca3a # v4.0.1
@@ -204,7 +207,7 @@ jobs:
           role-skip-session-tagging: true
           aws-region: us-west-2
           role-to-assume: ${{ vars.CONTAINER_IMAGE_UPLOAD_AWS_ROLE }}
-          role-session-name: "container-image-upload-${{ github.run_number }}"
+          role-session-name: "tag-build-container-image-upload-${{ github.run_attempt }}"
           role-duration-seconds: 900
       - name: Authenticate with ECR
         env:
diff --git a/.github/workflows/tag-publish.yaml b/.github/workflows/tag-publish.yaml
index 2f3ad586a..a8c1b4b02 100644
--- a/.github/workflows/tag-publish.yaml
+++ b/.github/workflows/tag-publish.yaml
@@ -16,9 +16,6 @@ on:
         description: "This field adds the provided value to a run step, allowing the calling actor to associate the started run with the GHA run ID."
         type: string
         required: false
-  release:
-    types:
-      - released  # This may need to be changed to "published"
 
 concurrency: "Limit to one build at a time for artifact tag ${{ inputs.artifact-tag || github.event.release.tag_name }}"
 
@@ -40,10 +37,10 @@ jobs:
         env:
           SEMVER_REGEX: ^v?(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)(?:-(?:(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+(?:[0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$
         run: |
-          (echo "$ARTIFACT_TAG" | grep -qP "$SEMVER_REGEX") || (echo "The artifact tag $ARTIFACT_TAG is not a valid semver-coerced value"; exit 1)
+          echo "$ARTIFACT_TAG" | grep -qP "$SEMVER_REGEX" || { echo "The artifact tag $ARTIFACT_TAG is not a valid semver-coerced value"; exit 1; }
       - name: Validate environment
         run: |
-          (echo "$ENVIRONMENT_NAME" | grep -qP '^publish-(prod|stage)$') || (echo "This workflow may only be ran from publishing environments"; exit 1)
+          echo "$ENVIRONMENT_NAME" | grep -qP '^publish-(prod|stage)$' || { echo "This workflow may only be ran from publishing environments"; exit 1; }
       - name: Checkout repo
         uses: actions/checkout@v4
         with:
@@ -61,12 +58,12 @@ jobs:
           role-skip-session-tagging: true
           aws-region: us-west-2
           role-to-assume: ${{ vars.ARTIFACT_DOWNLOAD_AWS_ROLE }}
-          role-session-name: "artifact-download-${{ github.run_number }}"
+          role-session-name: "tag-publish-artifact-download-${{ github.run_attempt }}"
           role-duration-seconds: 900
       - name: Download artifacts from S3
         env:
-          ARTIFACT_BUCKET: ${{ vars.ARTIFACT_SOURCE_BUCKET }}
-        run: aws s3 cp "s3://$ARTIFACT_BUCKET/teleport-plugins/tag/$ARTIFACT_TAG/" "$LOCAL_ARTIFACTS_PATH" --recursive
+          PENDING_BUCKET: ${{ vars.PENDING_BUCKET }}
+        run: aws s3 cp "s3://$PENDING_BUCKET/teleport-plugins/tag/$ARTIFACT_TAG/" "$LOCAL_ARTIFACTS_PATH" --recursive
       # Binary artifact promotion
       - name: Assume AWS role for uploading the artifacts
         # This step is only supported in production as there is no staging version of Houston
@@ -76,7 +73,7 @@ jobs:
           role-skip-session-tagging: true
           aws-region: us-west-2
           role-to-assume: ${{ vars.HOUSTON_UPLOAD_AWS_ROLE }}
-          role-session-name: "houston-upload-${{ github.run_number }}"
+          role-session-name: "tag-publish-houston-upload-${{ github.run_attempt }}"
           role-duration-seconds: 900
       - name: Upload artifacts to Houston
         # This step is only supported in production as there is no staging version of Houston
@@ -84,7 +81,7 @@ jobs:
         env:
           HOUSTON_BUCKET: ${{ vars.HOUSTON_BUCKET }}
         run: |
-          aws s3 sync --acl public-read "$LOCAL_ARTIFACTS_PATH" s3://$HOUSTON_BUCKET/teleport-plugins/${ARTIFACT_TAG##*-v}/ \
+          aws s3 sync --acl public-read "$LOCAL_ARTIFACTS_PATH" "s3://$HOUSTON_BUCKET/teleport-plugins/${ARTIFACT_TAG##v}/" \
             --include "*" \
             --exclude "*.tgz*"  # Exclude helm chart artifacts
       # Image promotion
@@ -94,7 +91,7 @@ jobs:
           role-skip-session-tagging: true
           aws-region: us-west-2
           role-to-assume: ${{ vars.CONTAINER_IMAGE_PUBLISHING_SYNC_AWS_ROLE }}
-          role-session-name: "container-image-publishing-sync-${{ github.run_number }}"
+          role-session-name: "tag-publish-container-image-publishing-sync-${{ github.run_attempt }}"
           role-duration-seconds: 900
       - name: Authenticate with ECR
         env:
@@ -105,7 +102,7 @@ jobs:
       - name: Publish access and event-handler images
         env:
           CONTAINER_IMAGE_PRIVATE_REGISTRY: ${{ vars.CONTAINER_IMAGE_PRIVATE_REGISTRY }}
-          CONTAINER_IMAGE_PUBLIC_REGISTRY: ${{ vars.CONTAINER_IMAGE_PUBLIC_REGISTRY}}
+          CONTAINER_IMAGE_PUBLIC_REGISTRY: ${{ vars.CONTAINER_IMAGE_PUBLIC_REGISTRY }}
           GITREF: ${{ env.ARTIFACT_TAG }}
         run: |
           export VERSION=${ARTIFACT_TAG##v}
@@ -118,7 +115,7 @@ jobs:
           role-skip-session-tagging: true
           aws-region: us-west-2
           role-to-assume: ${{ vars.HELM_PUBLISHING_SYNC_AWS_ROLE }}
-          role-session-name: "helm-publishing-sync-${{ github.run_number }}"
+          role-session-name: "tag-publish-helm-publishing-sync-${{ github.run_attempt }}"
           role-duration-seconds: 900
       - name: Download the Helm repo from S3
         env:
@@ -127,7 +124,7 @@ jobs:
       - name: Copy the Helm charts to the repo and regenerate the index
         working-directory: "${{ env.LOCAL_HELM_REPO_PATH }}"
         run: |
-          cp $(find "$LOCAL_ARTIFACTS_PATH" -name "teleport-plugin-*.tgz" -type f) .
+          find "$LOCAL_ARTIFACTS_PATH" -name 'teleport-plugin-*.tgz' -type f -exec cp {} "." \;
           helm repo index .
       - name: Upload the Helm repo to S3
         env:
@@ -140,7 +137,7 @@ jobs:
           role-skip-session-tagging: true
           aws-region: us-west-2
           role-to-assume: ${{ vars.TERRAFORM_PUBLISHING_SYNC_AWS_ROLE }}
-          role-session-name: "terraform-publishing-sync-${{ github.run_number }}"
+          role-session-name: "tag-publish-terraform-publishing-sync-${{ github.run_attempt }}"
           role-duration-seconds: 900
       - name: Download the Terraform repo from S3
         env:
diff --git a/tooling/bin/tf-promote-local.sh b/tooling/bin/tf-promote-local.sh
index cd084a651..00b085626 100755
--- a/tooling/bin/tf-promote-local.sh
+++ b/tooling/bin/tf-promote-local.sh
@@ -17,14 +17,12 @@ ARTIFACT_TAG="$1"
 ARTIFACT_BUCKET="$2"
 ARTIFACT_BUCKET_PROFILE="$3"
 ARTIFACT_BUCKET_PATH="s3://$ARTIFACT_BUCKET/teleport-plugins/tag/terraform-provider-teleport-$ARTIFACT_TAG/"
-# ARTIFACT_DIRECTORY=$(mktemp -d -t "terraform-promotion-artifacts")
-ARTIFACT_DIRECTORY="/var/folders/0c/lx19hczx3nqc6f_wh2rrmcp40000gn/T/terraform-promotion-artifacts.C5vvAsL8U9/" # TODO remove
+ARTIFACT_DIRECTORY=$(mktemp -d -t "terraform-promotion-artifacts")
 
 REGISTRY_BUCKET="$4"
 REGISTRY_BUCKET_PROFILE="$5"
 REGISTRY_BUCKET_PATH="s3://$REGISTRY_BUCKET/"
-# REGISTRY_DIRECTORY=$(mktemp -d -t "terraform-provider-registry")
-REGISTRY_DIRECTORY="/var/folders/0c/lx19hczx3nqc6f_wh2rrmcp40000gn/T/terraform-provider-registry.m1NA6ezAmm/" # TODO remove
+REGISTRY_DIRECTORY=$(mktemp -d -t "terraform-provider-registry")
 REGISTRY_URL="$6"
 
 echo "Downloading artifacts to $ARTIFACT_DIRECTORY from artifact storage bucket path $ARTIFACT_BUCKET_PATH with via $ARTIFACT_BUCKET_PROFILE profile"
diff --git a/tooling/cmd/promote-terraform/README.md b/tooling/cmd/promote-terraform/README.md
index 2a6135b44..4fd2ef024 100644
--- a/tooling/cmd/promote-terraform/README.md
+++ b/tooling/cmd/promote-terraform/README.md
@@ -1,4 +1,4 @@
 # What does this do?
 This tool takes a built Terraform provider tarball and packages it in the format expected by a Terraform repo. The tarball is expected to only contain the built provider binary itself. This tool converts it to a zip file, creates ".sum" and ".sum.sigs" files, then updates a local copy of an existing registry with the built file.
 
-It is up to external processes to create a local copy of the registry, and sync it to S3 if required.
\ No newline at end of file
+It is up to external processes to create a local copy of the registry, and sync it to S3 if required.