You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/05/03 06:08:42 UTC
[GitHub] [airflow-ci-infra] xinbinhuang opened a new pull request #20: Sync committers daily
xinbinhuang opened a new pull request #20:
URL: https://github.com/apache/airflow-ci-infra/pull/20
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow-ci-infra] xinbinhuang commented on a change in pull request #20: Sync committers daily
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #20:
URL: https://github.com/apache/airflow-ci-infra/pull/20#discussion_r625928879
##########
File path: lambdas/scale_out_runner/app.py
##########
@@ -20,15 +20,21 @@
import json
import logging
import os
+import re
from typing import cast
import boto3
+import requests
+import yaml
from chalice import BadRequestError, Chalice, ForbiddenError
-from chalice.app import Request
+from chalice.app import Rate, Request
app = Chalice(app_name='scale_out_runner')
app.log.setLevel(logging.INFO)
+GITHUB_CI_YML = 'https://raw.githubusercontent.com/apache/airflow/master/.github/workflows/ci.yml'
+SSM_REPO_NAME = os.getenv('SSM_REPO_NAME', 'apache/airflow')
+KMS_ID = os.getenv("KMS_ID", "airflow-ci")
Review comment:
Do I need to change the KMS id for SSM encryption? I don't have access to KMS.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow-ci-infra] xinbinhuang commented on a change in pull request #20: Sync committers daily
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #20:
URL: https://github.com/apache/airflow-ci-infra/pull/20#discussion_r624894663
##########
File path: lambdas/scale_out_runner/app.py
##########
@@ -101,12 +107,38 @@ def index():
return payload
-def commiters(ssm_repo_name: str = os.getenv('SSM_REPO_NAME', 'apache/airflow')):
+@app.schedule(Rate(1, unit=Rate.DAYS))
+def sync_committers(_):
+ app.log.info("Parsing committers list from %s", GITHUB_CI_YML)
+ resp = requests.get(GITHUB_CI_YML)
+ ci_yml = yaml.load(resp.text, yaml.SafeLoader)
+ runs_on = ci_yml['jobs']['build-info']['runs-on'].replace("\n", '')
+ committers = re.findall(r"fromJSON\('(\[.*\])'\)", runs_on)[0]
+
+ client = boto3.client('ssm')
+ param_path = get_ssm_param_path(SSM_REPO_NAME)
+
+ app.log.info("Updating config overlay for %s", param_path)
+ ssm_value = {"pullRequestSecurity": {"allowedAuthors": committers}}
Review comment:
I try to guess this part based on the func `commiters()`. Will refactor this with `committers()` if you can confirm this's the right SSM value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow-ci-infra] ashb commented on a change in pull request #20: Sync committers daily
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #20:
URL: https://github.com/apache/airflow-ci-infra/pull/20#discussion_r625701834
##########
File path: lambdas/scale_out_runner/app.py
##########
@@ -101,12 +107,38 @@ def index():
return payload
-def commiters(ssm_repo_name: str = os.getenv('SSM_REPO_NAME', 'apache/airflow')):
+@app.schedule(Rate(1, unit=Rate.DAYS))
+def sync_committers(_):
+ app.log.info("Parsing committers list from %s", GITHUB_CI_YML)
+ resp = requests.get(GITHUB_CI_YML)
+ ci_yml = yaml.load(resp.text, yaml.SafeLoader)
+ runs_on = ci_yml['jobs']['build-info']['runs-on'].replace("\n", '')
+ committers = re.findall(r"fromJSON\('(\[.*\])'\)", runs_on)[0]
+
+ client = boto3.client('ssm')
+ param_path = get_ssm_param_path(SSM_REPO_NAME)
+
+ app.log.info("Updating config overlay for %s", param_path)
+ ssm_value = {"pullRequestSecurity": {"allowedAuthors": committers}}
+ client.put_parameter(
+ Name=param_path,
+ Value=json.dumps(ssm_value),
Review comment:
There could be other things in there -- we should merge in to the existing value rather than just overwriting everything.
##########
File path: lambdas/scale_out_runner/app.py
##########
@@ -101,12 +107,38 @@ def index():
return payload
-def commiters(ssm_repo_name: str = os.getenv('SSM_REPO_NAME', 'apache/airflow')):
+@app.schedule(Rate(1, unit=Rate.DAYS))
+def sync_committers(_):
+ app.log.info("Parsing committers list from %s", GITHUB_CI_YML)
+ resp = requests.get(GITHUB_CI_YML)
Review comment:
```suggestion
resp = requests.get(GITHUB_CI_YML)
resp.raise_for_status()
```
##########
File path: lambdas/scale_out_runner/app.py
##########
@@ -101,12 +107,38 @@ def index():
return payload
-def commiters(ssm_repo_name: str = os.getenv('SSM_REPO_NAME', 'apache/airflow')):
+@app.schedule(Rate(1, unit=Rate.DAYS))
+def sync_committers(_):
+ app.log.info("Parsing committers list from %s", GITHUB_CI_YML)
+ resp = requests.get(GITHUB_CI_YML)
+ ci_yml = yaml.load(resp.text, yaml.SafeLoader)
+ runs_on = ci_yml['jobs']['build-info']['runs-on'].replace("\n", '')
+ committers = re.findall(r"fromJSON\('(\[.*\])'\)", runs_on)[0]
+
+ client = boto3.client('ssm')
+ param_path = get_ssm_param_path(SSM_REPO_NAME)
+
+ app.log.info("Updating config overlay for %s", param_path)
+ ssm_value = {"pullRequestSecurity": {"allowedAuthors": committers}}
Review comment:
Yes, this is the right value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow-ci-infra] ashb commented on pull request #20: Sync committers daily
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #20:
URL: https://github.com/apache/airflow-ci-infra/pull/20#issuecomment-832073852
Ah perfect.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow-ci-infra] xinbinhuang commented on a change in pull request #20: Sync committers daily
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #20:
URL: https://github.com/apache/airflow-ci-infra/pull/20#discussion_r625952645
##########
File path: lambdas/scale_out_runner/.chalice/prod/sync_committers.json
##########
@@ -0,0 +1,24 @@
+{
+ "Version": "2012-10-17",
+ "Statement": [
+ {
+ "Sid": "SyncCommitters",
+ "Effect": "Allow",
+ "Action": [
+ "kms:Decrypt",
+ "kms:Encrypt",
+ "kms:GenerateDataKey",
Review comment:
Not 100% sure if `kms:GenerateDataKey` is needed..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow-ci-infra] xinbinhuang commented on pull request #20: Sync committers daily
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on pull request #20:
URL: https://github.com/apache/airflow-ci-infra/pull/20#issuecomment-832073207
> This maybe should be a separate lambda -- that way the one that responds to WebHooks (i.e. is publically accessible at all) can not have permission to _update_ anything in SSM.
Good point!
Though it's indeed a separate lambda, I think they currently share the same IAM. I will update a [per lambda IAM role](https://aws.github.io/chalice/topics/configfile.html#per-lambda-examples) in the config
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow-ci-infra] ashb commented on pull request #20: Sync committers daily
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #20:
URL: https://github.com/apache/airflow-ci-infra/pull/20#issuecomment-832065791
This maybe should be a separate lambda -- that way the one that responds to WebHooks (i.e. is publically accessible at all) can not have permission to _update_ anything in SSM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow-ci-infra] xinbinhuang commented on a change in pull request #20: Sync committers daily
Posted by GitBox <gi...@apache.org>.
xinbinhuang commented on a change in pull request #20:
URL: https://github.com/apache/airflow-ci-infra/pull/20#discussion_r624894663
##########
File path: lambdas/scale_out_runner/app.py
##########
@@ -101,12 +107,38 @@ def index():
return payload
-def commiters(ssm_repo_name: str = os.getenv('SSM_REPO_NAME', 'apache/airflow')):
+@app.schedule(Rate(1, unit=Rate.DAYS))
+def sync_committers(_):
+ app.log.info("Parsing committers list from %s", GITHUB_CI_YML)
+ resp = requests.get(GITHUB_CI_YML)
+ ci_yml = yaml.load(resp.text, yaml.SafeLoader)
+ runs_on = ci_yml['jobs']['build-info']['runs-on'].replace("\n", '')
+ committers = re.findall(r"fromJSON\('(\[.*\])'\)", runs_on)[0]
+
+ client = boto3.client('ssm')
+ param_path = get_ssm_param_path(SSM_REPO_NAME)
+
+ app.log.info("Updating config overlay for %s", param_path)
+ ssm_value = {"pullRequestSecurity": {"allowedAuthors": committers}}
Review comment:
I guess this part based on the func `commiters()`. Will refactor this with `committers()` if you can confirm this's the right SSM value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org