Skip to content

Commit 2068c23

Browse files
committed
use PubSub JWT auth for security instead of the hardcoded static key
1 parent 5a67bbb commit 2068c23

16 files changed

+20
-22
lines changed

.gitignore

+6-1
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,18 @@
11

2-
# See config.yaml.original in git; the others are dynamic and so not put in git.
2+
# See config.yaml.original and cron_.*.yaml in git;
3+
# the below are dynamic and so not put in git.
34
config-test.yaml
45
config-dev.yaml
56
config.yaml
67
cron.yaml
8+
#
9+
710
#
811
__pycache__/
912
*.bak
1013
.DS_Store
14+
*.swp
15+
1116
TEMP*
1217
.idea/
1318

iris-custom-role.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ description: >
33
Permissions needed by the custom role used by the Iris App Engine app's service account.
44
A nondefault name for this role can be passed as env variable IRIS_CUSTOM_ROLE
55
into the deployment and uninstall scripts.
6-
# TODO For DRY, embed the following values in the Python class, then call these classes to construct this.
6+
# TODO Each Python plugin class should expose these and we should pull it from there,to have the info in one place.
77
#
88
# TODO: Not clear that `setTags` is really needed.
99
stage: "GA"

main.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def label_one():
235235

236236
def __check_pubsub_jwt():
237237
try:
238-
#TODO the sample https://github.com/GoogleCloudPlatform/python-docs-samples/blob/ff4c1d55bb5b6995c63383469535604002dc9ba2/appengine/standard_python3/pubsub/main.py#L69
238+
# TODO the sample https://github.com/GoogleCloudPlatform/python-docs-samples/blob/ff4c1d55bb5b6995c63383469535604002dc9ba2/appengine/standard_python3/pubsub/main.py#L69
239239
# has this. I am not sure why or how the token arg gets there.
240240
# if request.args.get("token", "") != current_app.config["PUBSUB_VERIFICATION_TOKEN"]:
241241
# return "Invalid request", 400

plugins/bigquery.py

-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ def _discovery_api():
2323
@lru_cache(maxsize=500) # cached per project
2424
def _cloudclient(cls, project_id=None):
2525
assert project_id, "'None' is only for the signature"
26-
logging.info("_cloudclient for %s", cls.__name__)
2726
# Local import to avoid burdening AppEngine memory.
2827
# Loading all Cloud Client libraries would be 100MB means that
2928
# the default AppEngine Instance crashes on out-of-memory even before actually serving a request.

plugins/buckets.py

-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ def method_names():
2020
@lru_cache(maxsize=500) # cached per project
2121
def _cloudclient(cls, project_id=None):
2222
assert project_id, "'None' is only for the signature"
23-
logging.info("_cloudclient for %s", cls.__name__)
2423
# Local import to avoid burdening AppEngine memory.
2524
# Loading all Cloud Client libraries would be 100MB means that
2625
# the default AppEngine Instance crashes on out-of-memory even before actually serving a request.

plugins/cloudsql.py

-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ def method_names():
1818

1919
@classmethod
2020
def _cloudclient(cls, _=None):
21-
logging.info("_cloudclient for %s", cls.__name__)
2221
raise NotImplementedError(
2322
"There is no Cloud Client library for " + cls.__name__
2423
)

plugins/disks.py

-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ class Disks(GceZonalBase):
2222
@staticmethod
2323
@lru_cache(maxsize=1)
2424
def _create_cloudclient():
25-
logging.info("_cloudclient for %s", "Disks")
2625
# Local import to avoid burdening AppEngine memory.
2726
# Loading all Cloud Client libraries would be 100MB means that
2827
# the default AppEngine Instance crashes on out-of-memory even before actually serving a request.

plugins/instances.py

-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ class Instances(GceZonalBase):
1717
@staticmethod
1818
@lru_cache(maxsize=1)
1919
def _create_cloudclient():
20-
21-
logging.info("_cloudclient for %s", "Instances")
2220
# Local import to avoid burdening AppEngine memory.
2321
# Loading all Cloud Client libraries would be 100MB means that
2422
# the default AppEngine Instance crashes on out-of-memory even before actually serving a request.

plugins/snapshots.py

-2
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ class Snapshots(GceBase):
1313
@classmethod
1414
@lru_cache(maxsize=1)
1515
def _cloudclient(cls, _=None):
16-
17-
logging.info("_cloudclient for %s", cls.__name__)
1816
# Local import to avoid burdening AppEngine memory. Loading all
1917
# Client libraries would be 100MB means that the default AppEngine
2018
# Instance crashes on out-of-memory even before actually serving a request.

plugins/subscriptions.py

-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ class Subscriptions(Plugin):
1717
@classmethod
1818
@lru_cache(maxsize=1)
1919
def _cloudclient(cls, _=None):
20-
logging.info("_cloudclient for %s", cls.__name__)
2120
# Local import to avoid burdening AppEngine memory.
2221
# Loading all Cloud Client libraries would be 100MB means that
2322
# the default AppEngine Instance crashes on out-of-memory even before actually serving a request.

plugins/topics.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class Topics(Plugin):
1717
@classmethod
1818
@lru_cache(maxsize=1)
1919
def _cloudclient(cls, _=None):
20-
logging.info("_cloudclient for %s", cls.__name__)
20+
2121
# Local import to avoid burdening AppEngine memory.
2222
# Loading all Cloud Client libraries would be 100MB means that
2323
# the default AppEngine Instance crashes on out-of-memory even before actually serving a request.

scripts/_deploy-org.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ else
7979
fi
8080

8181
# Add methodName filter to the log sink
82-
#TODO get this directly from the Python class to avoid duplication
82+
#TODO Each Python plugin class should expose these and we should pull it from there,to have the info in one place.
8383
log_filter+=('protoPayload.methodName:(')
8484
log_filter+=('"storage.buckets.create"')
8585
log_filter+=('OR "compute.instances.insert" OR "compute.instances.start" OR "datasetservice.insert"')

scripts/_deploy-project.sh

+2-3
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ set -e
114114
MSGSENDER_SERVICE_ACCOUNT=${msg_sender_sa_name}@${PROJECT_ID}.iam.gserviceaccount.com
115115

116116
gcloud projects add-iam-policy-binding ${PROJECT_ID} \
117-
--member="serviceAccount:${MSGSENDER_SERVICE_ACCOUNT}"\
117+
--member="serviceAccount:${PUBSUB_SERVICE_ACCOUNT}"\
118118
--role='roles/iam.serviceAccountTokenCreator'
119119

120120
set +e
@@ -173,10 +173,9 @@ fi
173173

174174
# Allow Pubsub to delete failed message from this sub
175175
gcloud pubsub subscriptions add-iam-policy-binding $DO_LABEL_SUBSCRIPTION \
176-
--member="serviceAccount:$PUBSUB_SERVICE_ACCOUNT"\
176+
--member="serviceAccount:$PUBSUB_SERVICE_ACCOUNT" \
177177
--role="roles/pubsub.subscriber" --project $PROJECT_ID >/dev/null
178178

179-
180179
if [[ "$LABEL_ON_CREATION_EVENT" != "true" ]]; then
181180
echo >&2 "Will not label on creation event."
182181
gcloud pubsub subscriptions delete "$LABEL_ONE_SUBSCRIPTION" --project="$PROJECT_ID" 2>/dev/null || true

test_scripts/integration_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ def pause_for_user_input():
294294
print("Type ENTER to proceed to clean up resources ")
295295
_ = sys.stdin.readline()
296296

297-
#pause_for_user_input()# Use this in debugging, to keep the test resources alive until you hit E
297+
# pause_for_user_input()# Use this in debugging, to keep the test resources alive until you hit E
298298

299299
remove_config_file()
300300
commands = [

uninstall_scripts/_uninstall-for-project.sh

+6-3
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,12 @@ gcloud pubsub subscriptions remove-iam-policy-binding $LABEL_ONE_SUBSCRIPTION \
3131
--member="serviceAccount:$PUBSUB_SERVICE_ACCOUNT"\
3232
--role="roles/pubsub.subscriber" --project $PROJECT_ID ||true
3333

34-
gcloud projects remove-iam-policy-binding ${PROJECT_ID} \
35-
--member="serviceAccount:${MSGSENDER_SERVICE_ACCOUNT}"\
36-
--role='roles/iam.serviceAccountTokenCreator' ||true
34+
# We don't do the following to avoid disrupting PubSub more generally in the proect.
35+
#gcloud projects remove-iam-policy-binding ${PROJECT_ID} \
36+
# --member="serviceAccount:${PUBSUB_SERVICE_ACCOUNT}"\
37+
# --role='roles/iam.serviceAccountTokenCreator' ||true
38+
39+
3740

3841
gcloud pubsub subscriptions delete $DEADLETTER_SUB --project="$PROJECT_ID" -q || true
3942
gcloud pubsub subscriptions delete "$DO_LABEL_SUBSCRIPTION" -q --project="$PROJECT_ID" ||true

util/utils.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ def run_command(command_s, extra_env: typing.Dict[str, str] = None):
252252
command = command_s.split(" ")
253253
env = os.environ
254254
if extra_env is not None:
255-
env = env | extra_env
255+
env |= extra_env
256256
result = subprocess.run(command, stdout=subprocess.PIPE, check=True, env=env)
257257
output = result.stdout.decode("utf-8")
258258
return output.strip("\n")

0 commit comments

Comments
 (0)