From 106edd48684c3d6b2d8868cb6b41d585decc7350 Mon Sep 17 00:00:00 2001
From: "Benjamin A. Petersen" <ben@benjaminapetersen.me>
Date: Thu, 12 Oct 2023 16:58:43 -0400
Subject: [PATCH] Add comments to build files

---
 hack/build-carvel-packages.sh         | 11 +++++++++--
 hack/noop.sh                          |  2 ++
 hack/prepare-for-integration-tests.sh |  1 +
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hack/build-carvel-packages.sh b/hack/build-carvel-packages.sh
index 77e214a4..cd6a50e8 100755
--- a/hack/build-carvel-packages.sh
+++ b/hack/build-carvel-packages.sh
@@ -69,6 +69,8 @@ fi
 pinniped_package_version="${tag}" # ie, "0.25.0"
 
 # core pinniped binaries (concierge, supervisor, local-user-authenticator)
+# TODO: we can likely just pass in the whole registry_repo_tag from the parent script and be done.
+#    the duplication is unnecessary.  This script doesn't ever need to run standalone again.
 registry="kind-registry.local:5000"
 repo="test/build"
 registry_repo="$registry/$repo"
@@ -191,6 +193,10 @@ do
   pinniped_package_rbac_prefix="pinniped-package-rbac-${resource_name}"
   pinniped_package_rbac_file="deploy_carvel/deploy/${pinniped_package_rbac_prefix}-${resource_name}-rbac.yml"
   echo -n "" > "${pinniped_package_rbac_file}"
+# TODO: will just a Role and RoleBinding work? Just for the target namespace.
+# - limit this to the LEAST privilege for each of the resources
+# - and document this for each of the resources.
+# - and we may need to TEMPLATE the namespace, if pinniped is installed in alt namespaces?
 cat <<EOF >> "${pinniped_package_rbac_file}"
 ---
 apiVersion: v1
@@ -208,7 +214,6 @@ kind: ClusterRole
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:
   name: "${pinniped_package_rbac_prefix}-role-superadmin-dangerous"
-  namespace: "${namespace}"
 rules:
 - apiGroups: ["*"]
   resources: ["*"]
@@ -278,7 +283,7 @@ kapp deploy --app "${KAPP_CONTROLLER_APP_NAME}" --file "${PACKAGE_INSTALL_FILE_N
 
 test_username="test-username"
 test_groups="test-group-0,test-group-1"
-test_password="$(openssl rand -hex 16)" # TODO: this will be different than in the prepare-for-integration-tests.sh file.
+test_password="$(openssl rand -hex 16)"
 log_note "Creating test user '$test_username'..."
 kubectl create secret generic "$test_username" \
   --namespace local-user-authenticator \
@@ -288,6 +293,7 @@ kubectl create secret generic "$test_username" \
   --output yaml |
   kubectl apply -f -
 
+# TODO: this is a race, we need to wait for this secret to exist, should we --wait?
 webhook_ca_bundle="$(kubectl get secret local-user-authenticator-tls-serving-certificate --namespace local-user-authenticator -o 'jsonpath={.data.caCertificate}')"
 # end local-user-authenticator
 
@@ -406,6 +412,7 @@ EOF
 
 KAPP_CONTROLLER_APP_NAME="${resource_name}-pkginstall"
 log_note "deploying ${KAPP_CONTROLLER_APP_NAME}..."
+# TODO: does this wait not only for the PackageInstall, but the Package, and its deployments and pods, to be successful?  Because we need that.
 kapp deploy --app "${KAPP_CONTROLLER_APP_NAME}" --file "${PACKAGE_INSTALL_FILE_NAME}" -y
 # end supervisor
 
diff --git a/hack/noop.sh b/hack/noop.sh
index 99511277..ff76a90f 100755
--- a/hack/noop.sh
+++ b/hack/noop.sh
@@ -47,6 +47,8 @@ app=${1}
 ## tag is fed in from the prepare-for-integration-tests.sh script, just uuidgen to identify a
 ## specific docker build of the pinniped-server image.
 tag=${2}
+# env_file_name is where to write env vars, if necessary to contribute to the environment
+env_file_name=${3}
 #SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 #log_note "noop.sh >>> script dir: ${SCRIPT_DIR}"
 log_note "noop.sh >>> app: ${app} tag: ${tag}"
diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh
index 02d9629f..4f806a71 100755
--- a/hack/prepare-for-integration-tests.sh
+++ b/hack/prepare-for-integration-tests.sh
@@ -318,6 +318,7 @@ else
     --output yaml |
     kubectl apply -f -
 
+  # TODO: this is a race, we need to wait for this secret to exist, should we --wait?
   webhook_ca_bundle="$(kubectl get secret local-user-authenticator-tls-serving-certificate --namespace local-user-authenticator -o 'jsonpath={.data.caCertificate}')"
   echo "export PINNIPED_TEST_WEBHOOK_CA_BUNDLE=${webhook_ca_bundle}" >> "${env_file_name}"
   popd >/dev/null