diff --git a/.github/labeler.yml b/.github/labeler.yml new file mode 100644 index 0000000000..05a534244b --- /dev/null +++ b/.github/labeler.yml @@ -0,0 +1,12 @@ +deploy-agent: + - changed-files: + - any-glob-to-any-file: deploy-agent/** +deploy-board: + - changed-files: + - any-glob-to-any-file: deploy-board/** +deploy-sentinel: + - changed-files: + - any-glob-to-any-file: deploy-sentinel/** +deploy-service: + - changed-files: + - any-glob-to-any-file: deploy-service/** diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml new file mode 100644 index 0000000000..7134e419e2 --- /dev/null +++ b/.github/workflows/labeler.yml @@ -0,0 +1,16 @@ +name: "Pull Request Labeler" + +on: + - pull_request_target + +jobs: + labeler: + permissions: + contents: read + pull-requests: write + runs-on: ubuntu-latest + steps: + - uses: actions/labeler@v5 + with: + repo-token: "${{ secrets.GITHUB_TOKEN }}" + sync-labels: true diff --git a/deploy-agent/deployd/download/download_helper.py b/deploy-agent/deployd/download/download_helper.py index 5d760ab0ed..823286ac28 100644 --- a/deploy-agent/deployd/download/download_helper.py +++ b/deploy-agent/deployd/download/download_helper.py @@ -22,11 +22,11 @@ class DownloadHelper(metaclass=abc.ABCMeta): - def __init__(self, url): + def __init__(self, url) -> None: self._url = url @staticmethod - def hash_file(file_path): + def hash_file(file_path) -> str: sha = hashlib.sha1() with open(file_path, "rb") as f: for chunk in iter(lambda: f.read(4096), b""): @@ -34,7 +34,7 @@ def hash_file(file_path): return sha.hexdigest() @staticmethod - def md5_file(file_path): + def md5_file(file_path) -> str: md5 = hashlib.md5() with open(file_path, "rb") as f: for chunk in iter(lambda: f.read(4096), b""): diff --git a/deploy-agent/deployd/download/download_helper_factory.py b/deploy-agent/deployd/download/download_helper_factory.py index d9b2c4ffc8..d483e76b7e 100644 --- a/deploy-agent/deployd/download/download_helper_factory.py +++ b/deploy-agent/deployd/download/download_helper_factory.py @@ -13,6 +13,7 @@ # limitations under the License. import logging +from typing import Optional from future.moves.urllib.parse import urlparse from boto.s3.connection import S3Connection @@ -28,7 +29,7 @@ class DownloadHelperFactory(object): @staticmethod - def gen_downloader(url, config): + def gen_downloader(url, config) -> Optional[S3DownloadHelper|LocalDownloadHelper|HTTPDownloadHelper]: url_parse = urlparse(url) if url_parse.scheme == 's3': aws_access_key_id = config.get_aws_access_key() diff --git a/deploy-agent/deployd/download/downloader.py b/deploy-agent/deployd/download/downloader.py index 8e2b51514c..baf0dfa7da 100644 --- a/deploy-agent/deployd/download/downloader.py +++ b/deploy-agent/deployd/download/downloader.py @@ -31,7 +31,7 @@ class Downloader(object): - def __init__(self, config, build, url, env_name): + def __init__(self, config, build, url, env_name) -> None: self._matcher = re.compile(r'^.*?[.](?Ptar\.gz|tar\.bz2|\w+)$') self._base_dir = config.get_builds_directory() self._build_name = env_name @@ -39,15 +39,15 @@ def __init__(self, config, build, url, env_name): self._url = url self._config = config - def _get_inner_extension(self, url): + def _get_inner_extension(self, url) -> str: outerExtension = self._get_extension(url) inverseExtLen = (len(outerExtension) * -1) - 1 return self._get_extension(url[:inverseExtLen]) - def _get_extension(self, url): + def _get_extension(self, url) -> str: return self._matcher.match(url).group('ext') - def download(self): + def download(self) -> int: extension = self._get_extension(self._url.lower()) local_fn = u'{}-{}.{}'.format(self._build_name, self._build, extension) local_full_fn = os.path.join(self._base_dir, local_fn) diff --git a/deploy-agent/deployd/download/gpg_helper.py b/deploy-agent/deployd/download/gpg_helper.py index d680352908..1a2d02faa3 100644 --- a/deploy-agent/deployd/download/gpg_helper.py +++ b/deploy-agent/deployd/download/gpg_helper.py @@ -22,7 +22,7 @@ class gpgHelper(object): @staticmethod - def decryptFile(source, destination): + def decryptFile(source, destination) -> int: download_cmd = ['gpg', '--batch', '--yes', '--output', destination, '--decrypt', source] log.info('Running command: {}'.format(' '.join(download_cmd))) error_code = Status.SUCCEEDED diff --git a/deploy-agent/deployd/download/http_download_helper.py b/deploy-agent/deployd/download/http_download_helper.py index 43148099ed..1bdfa3198d 100644 --- a/deploy-agent/deployd/download/http_download_helper.py +++ b/deploy-agent/deployd/download/http_download_helper.py @@ -29,11 +29,11 @@ class HTTPDownloadHelper(DownloadHelper): - def __init__(self, url=None, config=None): + def __init__(self, url=None, config=None) -> None: super().__init__(url) self._config = config if config else Config() - def _download_files(self, local_full_fn): + def _download_files(self, local_full_fn) -> int: download_cmd = ['curl', '-o', local_full_fn, '-fksS', self._url] log.info('Running command: {}'.format(' '.join(download_cmd))) output, error, process_return_code = Caller.call_and_log(download_cmd, cwd=os.getcwd()) @@ -45,7 +45,7 @@ def _download_files(self, local_full_fn): log.info('Finish downloading: {} to {}'.format(self._url, local_full_fn)) return status_code - def download(self, local_full_fn): + def download(self, local_full_fn) -> int: log.info("Start to download from url {} to {}".format( self._url, local_full_fn)) if not self.validate_source(): @@ -76,7 +76,7 @@ def download(self, local_full_fn): log.error('Could not connect to: {}'.format(self._url)) return Status.FAILED - def validate_source(self): + def validate_source(self) -> bool: tags = {'type': 'http', 'url': self._url} create_sc_increment(DOWNLOAD_VALIDATE_METRICS, tags=tags) diff --git a/deploy-agent/deployd/download/local_download_helper.py b/deploy-agent/deployd/download/local_download_helper.py index 47d1188d42..1ca6cbac12 100644 --- a/deploy-agent/deployd/download/local_download_helper.py +++ b/deploy-agent/deployd/download/local_download_helper.py @@ -23,7 +23,7 @@ class LocalDownloadHelper(DownloadHelper): - def _download_files(self, local_full_fn): + def _download_files(self, local_full_fn) -> int: download_cmd = ['curl', '-o', local_full_fn, '-ks', self._url] log.info('Running command: {}'.format(' '.join(download_cmd))) error_code = Status.SUCCEEDED @@ -37,7 +37,7 @@ def _download_files(self, local_full_fn): log.info('Finished downloading: {} to {}'.format(self._url, local_full_fn)) return error_code - def download(self, local_full_fn): + def download(self, local_full_fn) -> int: log.info("Start to download from local path {} to {}".format( self._url, local_full_fn)) @@ -46,5 +46,5 @@ def download(self, local_full_fn): log.error('Failed to download the local tar ball for {}'.format(local_full_fn)) return error - def validate_source(self): - return True \ No newline at end of file + def validate_source(self) -> bool: + return True diff --git a/deploy-agent/deployd/download/s3_download_helper.py b/deploy-agent/deployd/download/s3_download_helper.py index d1ff0d8458..479c6c09e6 100644 --- a/deploy-agent/deployd/download/s3_download_helper.py +++ b/deploy-agent/deployd/download/s3_download_helper.py @@ -27,7 +27,7 @@ class S3DownloadHelper(DownloadHelper): - def __init__(self, local_full_fn, aws_connection=None, url=None): + def __init__(self, local_full_fn, aws_connection=None, url=None) -> None: super(S3DownloadHelper, self).__init__(local_full_fn) self._s3_matcher = "^s3://(?P[a-zA-Z0-9\-_]+)/(?P[a-zA-Z0-9\-_/\.]+)/?" self._config = Config() @@ -45,7 +45,7 @@ def __init__(self, local_full_fn, aws_connection=None, url=None): self._key = s3url_parse.group("KEY") - def download(self, local_full_fn): + def download(self, local_full_fn) -> int: log.info(f"Start to download file {self._key} from s3 bucket {self._bucket_name} to {local_full_fn}") if not self.validate_source(): log.error(f'Invalid url: {self._url}. Skip downloading.') @@ -76,9 +76,9 @@ def download(self, local_full_fn): log.error("Failed to get package from s3: {}".format(traceback.format_exc())) return Status.FAILED - def validate_source(self): + def validate_source(self) -> bool: allow_list = self._config.get_s3_download_allow_list() tags = {'type': 's3', 'url': self._url, 'bucket' : self._bucket_name} create_sc_increment(DOWNLOAD_VALIDATE_METRICS, tags=tags) - return self._bucket_name in allow_list if allow_list else True \ No newline at end of file + return self._bucket_name in allow_list if allow_list else True diff --git a/deploy-service/common/src/main/java/com/pinterest/deployservice/common/KnoxKeyReader.java b/deploy-service/common/src/main/java/com/pinterest/deployservice/common/KnoxKeyReader.java index dcf7826f29..d8c2a70318 100644 --- a/deploy-service/common/src/main/java/com/pinterest/deployservice/common/KnoxKeyReader.java +++ b/deploy-service/common/src/main/java/com/pinterest/deployservice/common/KnoxKeyReader.java @@ -73,6 +73,10 @@ private String getKeyInternal() { LOG.warn("Using default key since knoxManager is null"); return defaultKeyContent; } - return knoxManager.getKey(); + String key = knoxManager.getKey(); + if (key != null) { + return key.trim(); + } + return key; } } diff --git a/deploy-service/common/src/test/java/com/pinterest/deployservice/common/KnoxKeyReaderTest.java b/deploy-service/common/src/test/java/com/pinterest/deployservice/common/KnoxKeyReaderTest.java index bda5f0fb7b..0cc3be6af7 100644 --- a/deploy-service/common/src/test/java/com/pinterest/deployservice/common/KnoxKeyReaderTest.java +++ b/deploy-service/common/src/test/java/com/pinterest/deployservice/common/KnoxKeyReaderTest.java @@ -5,28 +5,34 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.only; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import org.junit.Before; import org.junit.Test; public class KnoxKeyReaderTest { + private KnoxKeyReader sut; + private KnoxKeyManager mockKnoxKeyManager; + + @Before + public void setUp() { + sut = new KnoxKeyReader(); + mockKnoxKeyManager = mock(KnoxKeyManager.class); + } + @Test public void testGetKey_noInit() { - KnoxKeyReader sut = new KnoxKeyReader(); assertEquals("defaultKeyContent", sut.getKey()); } @Test public void testGetKey_initialized() { - KnoxKeyReader sut = new KnoxKeyReader(); sut.init("someRandomKey"); assertNull(sut.getKey()); } @Test public void testGetKey_cacheIsWorking() { - KnoxKeyReader sut = new KnoxKeyReader(); - KnoxKeyManager mockKnoxKeyManager = mock(KnoxKeyManager.class); - sut.init("someRandomKey"); sut.setKnoxManager(mockKnoxKeyManager); sut.getKey(); @@ -34,4 +40,16 @@ public void testGetKey_cacheIsWorking() { verify(mockKnoxKeyManager, only()).getKey(); } + + @Test + public void testGetKey_trimmed() { + String expected = "keyWith234!~@"; + when(mockKnoxKeyManager.getKey()).thenReturn(String.format(" \n\r\t %s \n\r\t", expected)); + + sut.init("someRandomKey"); + sut.setKnoxManager(mockKnoxKeyManager); + String trimmed = sut.getKey(); + + assertEquals(expected, trimmed); + } } diff --git a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java index b612563379..892d6e65b9 100644 --- a/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java +++ b/deploy-service/teletraanservice/src/main/java/com/pinterest/teletraan/TeletraanService.java @@ -176,6 +176,8 @@ public void run(TeletraanServiceConfiguration configuration, Environment environ // Exception handlers // Constrains validation exceptions, returns 4xx environment.jersey().register(new JerseyViolationExceptionMapper()); + // Jackson Json parsing exceptions + environment.jersey().register(new JsonProcessingExceptionMapper(true)); environment.jersey().register(new GenericExceptionMapper(configuration.getSystemFactory().getClientError())); // Swagger API docs generation related