Skip to content

Commit efc5065

Browse files
Fix: Strengthen GMA skip logic in restore_secrets.py
This commit further refines `scripts/gha/restore_secrets.py` to prevent attempts to write to the deleted `gma/integration_test` directory. Changes include: - Initializing `dest_paths` as an empty list. - More careful construction of `dest_paths` based on whether `FLAGS.artifact` is set, ensuring that GMA paths are not inadvertently created. - Explicitly continuing the loop if no valid destination path is determined for a file, preventing unnecessary decryption or write attempts. - Adding redundant checks to ensure GMA paths are not processed before file write operations as a final safeguard. This should robustly address the `FileNotFoundError` previously observed in CI.
1 parent 1159716 commit efc5065

File tree

1 file changed

+57
-17
lines changed

1 file changed

+57
-17
lines changed

scripts/gha/restore_secrets.py

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -86,39 +86,79 @@ def main(argv):
8686
# /scripts/gha-encrypted/auth/google-services.json.gpg turns into
8787
# /<repo_dir>/auth/integration_test/google-services.json
8888
api = os.path.basename(os.path.dirname(path))
89-
if api == "gma": # Skip processing for GMA secrets
89+
# This is the primary skip for GMA. Ensure it's effective.
90+
if api == "gma":
9091
continue
92+
9193
if FLAGS.apis and api not in FLAGS.apis:
9294
print("Skipping secret found in product api", api)
9395
continue
96+
9497
print("Encrypted Google Service file found: %s" % path)
9598
file_name = os.path.basename(path).replace(".gpg", "")
96-
dest_paths = [os.path.join(repo_dir, api, "integration_test", file_name)]
99+
100+
# Initialize dest_paths. It will be populated based on conditions.
101+
dest_paths = []
102+
97103
if FLAGS.artifact:
98-
# /<repo_dir>/<artifact>/auth/google-services.json
99-
if "google-services" in path and os.path.isdir(os.path.join(repo_dir, FLAGS.artifact, api)):
100-
dest_paths = [os.path.join(repo_dir, FLAGS.artifact, api, file_name)]
104+
# If processing for artifacts, construct artifact-specific path.
105+
# The 'api == "gma"' check above should prevent this block from executing for GMA.
106+
artifact_api_path = os.path.join(repo_dir, FLAGS.artifact, api)
107+
if "google-services" in path and os.path.isdir(artifact_api_path):
108+
dest_paths.append(os.path.join(artifact_api_path, file_name))
101109
else:
110+
# If not a google-services file or the artifact API directory doesn't exist,
111+
# skip this file for artifact processing.
112+
print(f"Skipping artifact path for {path} (API: {api}, Artifact: {FLAGS.artifact})")
113+
# This continue is vital: if no artifact path, don't fall through to default path logic for this file.
102114
continue
115+
else:
116+
# If not processing for artifacts, use the default integration_test path.
117+
# The 'api == "gma"' check above should prevent this line from executing for GMA.
118+
dest_paths.append(os.path.join(repo_dir, api, "integration_test", file_name))
119+
120+
# If dest_paths is still empty (e.g., was artifact flow but conditions not met), skip.
121+
if not dest_paths:
122+
print(f"No valid destination paths determined for {path} (API: {api}). Skipping.")
123+
continue
124+
125+
# Append internal integration test path if it exists, for non-GMA APIs.
126+
# This now correctly checks 'api' before forming the path.
127+
if api != "gma" and os.path.exists(os.path.join(repo_dir, api, "integration_test_internal")):
128+
# Only add internal path if a primary path was already added.
129+
# This ensures we don't *only* try to write to an internal path if, for instance,
130+
# the artifact conditions weren't met but an internal dir exists.
131+
if dest_paths: # Check if dest_paths has at least one path already
132+
dest_paths.append(os.path.join(repo_dir, api, "integration_test_internal", file_name))
133+
134+
# If, after all checks, dest_paths is empty, skip.
135+
if not dest_paths:
136+
print(f"No destination paths after internal check for {path}. Skipping.")
137+
continue
103138

104-
# Some apis like Firestore also have internal integration tests.
105-
if os.path.exists( os.path.join(repo_dir, api, "integration_test_internal")):
106-
dest_paths.append(os.path.join(repo_dir, api,
107-
"integration_test_internal", file_name))
108139
decrypted_text = _decrypt(path, passphrase)
109-
for dest_path in dest_paths:
110-
with open(dest_path, "w") as f:
140+
for dest_path_item in dest_paths:
141+
# Final explicit check, although redundant if above logic is correct.
142+
if "/gma/" in dest_path_item or "\\gma\\" in dest_path_item:
143+
print(f"CRITICAL WARNING: Attempting to write to a GMA path, this should not happen: {dest_path_item}. Skipping.")
144+
continue
145+
print(f"Writing decrypted file to {dest_path_item}")
146+
with open(dest_path_item, "w") as f:
111147
f.write(decrypted_text)
112-
print("Copied decrypted google service file to %s" % dest_path)
113-
# We use a Google Service file as the source of truth for the reverse id
114-
# that needs to be patched into the Info.plist files.
115-
if dest_path.endswith(".plist"):
116-
_patch_reverse_id(dest_path)
117-
_patch_bundle_id(dest_path)
148+
print("Copied decrypted google service file to %s" % dest_path_item)
149+
if dest_path_item.endswith(".plist"):
150+
_patch_reverse_id(dest_path_item)
151+
_patch_bundle_id(dest_path_item)
152+
153+
# The rest of the script handles non-GoogleService files (uri_prefix, app_check_token, gcs_key_file)
154+
# These are not per-API, so the GMA removal shouldn't affect them directly,
155+
# as long as their respective directories still exist if they are not GMA-specific.
118156

157+
# This check should be fine as FLAGS.artifact is global to the script run.
119158
if FLAGS.artifact:
120159
return
121160

161+
# This part is for dynamic_links, not gma.
122162
if not FLAGS.apis or "dynamic_links" in FLAGS.apis:
123163
print("Attempting to patch Dynamic Links uri prefix.")
124164
uri_path = os.path.join(secrets_dir, "dynamic_links", "uri_prefix.txt.gpg")

0 commit comments

Comments
 (0)