SameerMesiah97 commented on code in PR #69886: URL: https://github.com/apache/airflow/pull/69886#discussion_r3589679340
########## providers/amazon/docs/operators/ecr.rst: ########## @@ -0,0 +1,87 @@ + .. Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + .. http://www.apache.org/licenses/LICENSE-2.0 + + .. Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + +======================================= +Amazon Elastic Container Registry (ECR) +======================================= + +`Amazon Elastic Container Registry (ECR) <https://aws.amazon.com/ecr/>`__ is a managed container registry +for storing, sharing, and deploying container images and artifacts. + +Prerequisite Tasks +------------------ + +.. include:: ../_partials/prerequisite_tasks.rst + +Generic Parameters +------------------ + +.. include:: ../_partials/generic_parameters.rst + +Operators +--------- + +.. _howto/operator:EcrCreateRepositoryOperator: + +Create an ECR repository +======================== + +To create an ECR repository, use +:class:`~airflow.providers.amazon.aws.operators.ecr.EcrCreateRepositoryOperator`. +The operator returns the complete response from the Boto3 ``create_repository`` API operation. + +.. exampleinclude:: /../../amazon/tests/system/amazon/aws/example_ecr.py + :language: python + :dedent: 4 + :start-after: [START howto_operator_ecr_create_repository] + :end-before: [END howto_operator_ecr_create_repository] + +.. _howto/operator:EcrSetRepositoryPolicyOperator: + +Set an ECR repository policy +============================ + +To set the repository policy for an ECR repository, use +:class:`~airflow.providers.amazon.aws.operators.ecr.EcrSetRepositoryPolicyOperator`. +The operator returns the complete response from the Boto3 ``set_repository_policy`` API operation. + +.. exampleinclude:: /../../amazon/tests/system/amazon/aws/example_ecr.py + :language: python + :dedent: 4 + :start-after: [START howto_operator_ecr_set_repository_policy] + :end-before: [END howto_operator_ecr_set_repository_policy] + +.. _howto/operator:EcrDeleteRepositoryOperator: + +Delete an ECR repository +======================== + +To delete an ECR repository, use +:class:`~airflow.providers.amazon.aws.operators.ecr.EcrDeleteRepositoryOperator`. +Set ``force=True`` to delete a repository that contains images. The operator returns the complete response +from the Boto3 ``delete_repository`` API operation. + +.. exampleinclude:: /../../amazon/tests/system/amazon/aws/example_ecr.py + :language: python + :dedent: 4 + :start-after: [START howto_operator_ecr_delete_repository] + :end-before: [END howto_operator_ecr_delete_repository] + +Reference +--------- + +* `AWS Boto3 library documentation for ECR <https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/ecr.html>`__ Review Comment: Non-blocking suggestion: would it be worth adding a sentence or two describing the typical use case for each operator rather than only describing the underlying Boto3 API call? I think that would make the documentation a bit more user-oriented. ########## providers/amazon/src/airflow/providers/amazon/aws/operators/ecr.py: ########## @@ -0,0 +1,233 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Amazon Elastic Container Registry (ECR) operators.""" Review Comment: I am not sure why this is needed. I know some of the operator modules start with a descriptive docstring like this but for these operators, it is self-evident what they arefrom how they are named. Also, ECR is not a niche AWS service. I would remove it. ########## providers/amazon/src/airflow/providers/amazon/aws/operators/ecr.py: ########## @@ -0,0 +1,233 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Amazon Elastic Container Registry (ECR) operators.""" + +from __future__ import annotations + +from collections.abc import Sequence +from typing import TYPE_CHECKING, Any, ClassVar + +from airflow.providers.amazon.aws.hooks.ecr import EcrHook +from airflow.providers.amazon.aws.operators.base_aws import AwsBaseOperator +from airflow.providers.amazon.aws.utils.mixins import aws_template_fields +from airflow.utils.helpers import prune_dict + +if TYPE_CHECKING: + from airflow.sdk import Context + + +class EcrCreateRepositoryOperator(AwsBaseOperator[EcrHook]): + """ + Create an Amazon ECR repository. + + .. seealso:: + For more information on how to use this operator, take a look at the guide: + :ref:`howto/operator:EcrCreateRepositoryOperator` + + :param repository_name: The name of the repository to create. (templated) + :param registry_id: The AWS account ID associated with the registry. (templated) + :param tags: Metadata to apply to the repository. (templated) + :param image_tag_mutability: The tag mutability setting for the repository. (templated) + :param image_tag_mutability_exclusion_filters: Filters that override the repository's + image tag mutability setting. (templated) + :param image_scanning_configuration: The image scanning configuration for the repository. (templated) + :param encryption_configuration: The encryption configuration for the repository. (templated) + :param aws_conn_id: The Airflow connection used for AWS credentials. + If this is ``None`` or empty then the default boto3 behaviour is used. If + running Airflow in a distributed manner and aws_conn_id is None or + empty, then default boto3 configuration would be used (and must be + maintained on each worker node). + :param region_name: AWS region_name. If not specified then the default boto3 behaviour is used. + :param verify: Whether or not to verify SSL certificates. See: + https://boto3.amazonaws.com/v1/documentation/api/latest/reference/core/session.html + :param botocore_config: Configuration dictionary (key-values) for botocore client. See: + https://botocore.amazonaws.com/v1/documentation/api/latest/reference/config.html + """ + + aws_hook_class = EcrHook + template_fields: Sequence[str] = aws_template_fields( + "repository_name", + "registry_id", + "tags", + "image_tag_mutability", + "image_tag_mutability_exclusion_filters", + "image_scanning_configuration", + "encryption_configuration", + ) + template_fields_renderers: ClassVar[dict[str, str]] = { + "tags": "json", + "image_tag_mutability_exclusion_filters": "json", + "image_scanning_configuration": "json", + "encryption_configuration": "json", + } + + def __init__( + self, + *, + repository_name: str, + registry_id: str | None = None, + tags: list[dict[str, str]] | None = None, + image_tag_mutability: str | None = None, + image_tag_mutability_exclusion_filters: list[dict[str, str]] | None = None, + image_scanning_configuration: dict[str, bool] | None = None, + encryption_configuration: dict[str, str] | None = None, + aws_conn_id: str | None = "aws_default", + **kwargs, + ): + super().__init__(aws_conn_id=aws_conn_id, **kwargs) + self.repository_name = repository_name + self.registry_id = registry_id + self.tags = tags + self.image_tag_mutability = image_tag_mutability + self.image_tag_mutability_exclusion_filters = image_tag_mutability_exclusion_filters + self.image_scanning_configuration = image_scanning_configuration + self.encryption_configuration = encryption_configuration + + def execute(self, context: Context) -> dict[str, Any]: + self.log.info("Creating Amazon ECR repository %s", self.repository_name) + response = self.hook.conn.create_repository( + **prune_dict( + { + "registryId": self.registry_id, + "repositoryName": self.repository_name, + "tags": self.tags, + "imageTagMutability": self.image_tag_mutability, + "imageTagMutabilityExclusionFilters": self.image_tag_mutability_exclusion_filters, + "imageScanningConfiguration": self.image_scanning_configuration, + "encryptionConfiguration": self.encryption_configuration, + } + ) + ) + self.log.info("Created Amazon ECR repository %s", self.repository_name) + return response Review Comment: Is it necessary to return the entire response? What does the payload look like? If it is very large with lots of unnecessary context, maybe you could parse out the repository URI/ARN/name and then return that. Same applies to all of the other operators. ########## providers/amazon/src/airflow/providers/amazon/aws/operators/ecr.py: ########## @@ -0,0 +1,233 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Amazon Elastic Container Registry (ECR) operators.""" + +from __future__ import annotations + +from collections.abc import Sequence +from typing import TYPE_CHECKING, Any, ClassVar + +from airflow.providers.amazon.aws.hooks.ecr import EcrHook +from airflow.providers.amazon.aws.operators.base_aws import AwsBaseOperator +from airflow.providers.amazon.aws.utils.mixins import aws_template_fields +from airflow.utils.helpers import prune_dict + +if TYPE_CHECKING: + from airflow.sdk import Context + + +class EcrCreateRepositoryOperator(AwsBaseOperator[EcrHook]): + """ + Create an Amazon ECR repository. + + .. seealso:: + For more information on how to use this operator, take a look at the guide: + :ref:`howto/operator:EcrCreateRepositoryOperator` + + :param repository_name: The name of the repository to create. (templated) + :param registry_id: The AWS account ID associated with the registry. (templated) + :param tags: Metadata to apply to the repository. (templated) + :param image_tag_mutability: The tag mutability setting for the repository. (templated) + :param image_tag_mutability_exclusion_filters: Filters that override the repository's + image tag mutability setting. (templated) + :param image_scanning_configuration: The image scanning configuration for the repository. (templated) + :param encryption_configuration: The encryption configuration for the repository. (templated) + :param aws_conn_id: The Airflow connection used for AWS credentials. + If this is ``None`` or empty then the default boto3 behaviour is used. If + running Airflow in a distributed manner and aws_conn_id is None or + empty, then default boto3 configuration would be used (and must be + maintained on each worker node). + :param region_name: AWS region_name. If not specified then the default boto3 behaviour is used. + :param verify: Whether or not to verify SSL certificates. See: + https://boto3.amazonaws.com/v1/documentation/api/latest/reference/core/session.html + :param botocore_config: Configuration dictionary (key-values) for botocore client. See: + https://botocore.amazonaws.com/v1/documentation/api/latest/reference/config.html Review Comment: 1) I think indicating whether each parameter is templated in the docstrings looks noisy. I would remove '(templated)' from each docstring entry unless you have a solid reason. 2) I would add the default value for each paramter at the end of its associated docstring entry. ########## providers/amazon/tests/system/amazon/aws/example_ecr.py: ########## @@ -0,0 +1,99 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +from datetime import datetime + +from airflow.providers.amazon.aws.operators.ecr import ( + EcrCreateRepositoryOperator, + EcrDeleteRepositoryOperator, + EcrSetRepositoryPolicyOperator, +) +from airflow.providers.common.compat.sdk import DAG, chain + +from system.amazon.aws.utils import ENV_ID_KEY, SystemTestContextBuilder +from tests_common.test_utils.version_compat import AIRFLOW_V_3_0_PLUS + +if AIRFLOW_V_3_0_PLUS: + from airflow.sdk import TriggerRule +else: + from airflow.utils.trigger_rule import TriggerRule # type: ignore[no-redef,attr-defined] + +DAG_ID = "example_ecr" + +sys_test_context_task = SystemTestContextBuilder().build() + + +with DAG( + dag_id=DAG_ID, + schedule=None, + start_date=datetime(2024, 1, 1), + catchup=False, +) as dag: + test_context = sys_test_context_task() + repository_name = f"{test_context[ENV_ID_KEY]}-test-repository" + + # [START howto_operator_ecr_create_repository] + create_repository = EcrCreateRepositoryOperator( + task_id="create_repository", + repository_name=repository_name, + ) + # [END howto_operator_ecr_create_repository] + + # [START howto_operator_ecr_set_repository_policy] + set_repository_policy = EcrSetRepositoryPolicyOperator( + task_id="set_repository_policy", + repository_name=repository_name, + policy_text=""" + { + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "AllowAccountPull", + "Effect": "Allow", + "Principal": { + "AWS": "arn:aws:iam::{{ task_instance.xcom_pull(task_ids='create_repository')['repository']['registryId'] }}:root" Review Comment: is the registryId lookup via XCom needed in this example? Since the repository is created in the default registry, omitting it would make the example a little simpler while still demonstrating the operator lifecycle -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
