Merge pull request #1700 from TrilokGeer/tags-addendum

CFE-1162: Updates enhancement to reflect hcp usecase and with latest information on aws tags support

Author: openshift-merge-bot[bot]

enhancements/api-review/custom-tags-aws.md

@@ -39,10 +39,10 @@ status: implementable
 This enhancement describes a proposal to allow an administrator of OpenShift to
 have the ability to apply user defined tags to many resources created by OpenShift in AWS.
 
-Note: this enhancement is slightly retroactive. Work has already begun on this. See
-- https://github.com/openshift/api/pull/864
-- https://github.com/openshift/cluster-ingress-operator/pull/578
-- https://github.com/openshift/api/pull/1064/
+Note: The work is carried out as part of 
+- https://issues.redhat.com/browse/RFE-1101
+- https://issues.redhat.com/browse/RFE-2012
+- https://issues.redhat.com/browse/RFE-3677
 
 ## Motivation
 
@@ -62,22 +62,21 @@ list of user-defined tags to the OpenShift Installer, and everything created by
 bootstrapped components will apply those tags to all resources created in AWS, for the life of the cluster, and where supported by AWS.
 - Tags must be applied at creation time, in an atomic operation. It isn't acceptable to create an object and,
 after some period of time, apply the tags post-creation.
+- Tags can be updated day2 on the AWS resources.
 
 ### Non-Goals
 
-- To reduce initial scope, tags are applied only at creation time and not reconciled. If an administrator manually
-changes the tags stored in the infrastructure resource, behavior is undefined. See below.
-- To reduce initial scope, we are not implementing this for clouds other than AWS. We will not take any actions
+- To reduce initial scope, in case of standalone OpenShift, tags are applied only at creation time and not reconciled. If an administrator manually
+changes the tags stored in the infrastructure resource, behavior is undefined. 
+- To reduce initial scope, for hosted control plane deployments, we are not implementing this for clouds other than AWS. We will not take any actions
 to prohibit that later.
 
 ## Proposal
 
-### Existing design details
-
-New `experimentalPropagateUserTags` field added to `.platform.aws` of install config to indicate that the user tags should be applied to AWS
+New `propagateUserTags` field added to `.platform.aws` of install config to indicate that the user tags should be applied to AWS
 resources created by in-cluster operators.
 
-If `experimentalPropagateUserTags` is true, install validation will fail if there is any tag that starts with `kubernetes.io` or `openshift.io`.
+If `propagateUserTags` is true, install validation will fail if there is any tag that starts with `kubernetes.io` or `openshift.io`.
 
 Add a new field `resourceTags` to `.status.aws` of the `infrastructure.config.openshift.io` type. Tags included in the
 `resourceTags` field will be applied to new resources created for the cluster. The `resourceTags` field will be populated by the installer only if the `experimentalPropagateUserTags` field is true.
@@ -91,19 +90,20 @@ userTags that are specified in the infrastructure resource will merge with userT
 
 The userTags field is intended to be set at install time and is considered immutable. Components that respect this field must only ever add tags that they retrieve from this field to cloud resources, they must never remove tags from the existing underlying cloud resource even if the tags are removed from this field(despite it being immutable).
 
-If the userTags field is changed post-install, there is no guarantee about how an in-cluster operator will respond to the change. Some operators may reconcile the change and change tags on the AWS resource. Some operators may ignore the change. However, if tags are removed from userTags, the tag will not be removed from the AWS resource.
-
-### Updated design details
+In case of hosted control plane deployments, the userTags field is updated with latest updates requested by user. The merge logic gives higher precedence to userTags when there is duplicate tag found on the AWS resource.
 
-A new field `propagateUserTags` is added in GA release version. The `experimentalPropagateUserTags` field will be deprecated. In future release versions, `experimentalPropagateUserTags` will be removed.
-When both fields are set, `experimentalPropagateUserTags` takes precedence.
+If the userTags field is changed post-install, all AWS resources created and managed by in-cluster and RedHat supported operators will be reconciled. Non-redhat supported operators may reconcile the change and change tags on the AWS resource or may ignore the change. However, if tags are removed from userTags, the tag will not be removed from the AWS resource.
 
+For the resources created and managed by hosted control plane, cluster api provider for aws reconciles the user tags on AWS resources. The hosted control plane updates the `infrastructure.config.openshift.io` resource to reflect new tags in `resourceTags`. The OpenShift operators, both core and non-core (managed by RedHat), reconcile the respective AWS resources created and managed by them. 
+Given that, there is no universal controller to update all resources created by OpenShift, the day2 updates of tags is not supported for standalone OpenShift deployments.
 ### User Stories
 
-https://issues.redhat.com/browse/SDE-1146 - IAM users and roles can only operate on resources with specific tags
+1. https://issues.redhat.com/browse/SDE-1146 - IAM users and roles can only operate on resources with specific tags
 As a security-conscious ROSA customer, I want to restrict the permissions granted to Red Hat in my AWS account by using
 AWS resource tags.
 
+2. https://issues.redhat.com/browse/OCPSTRAT-787 - As a user of ROSA with HCP, I want to add and update tags on all AWS resources created by OpenShift, given that there is security limitation on direct access to update to AWS resources.
+
 ### API Extensions
 
 ```yaml
@@ -119,17 +119,14 @@ apiVersion: apiextensions.k8s.io/v1
           properties:
             platform:
               aws:
-                experimentalPropagateUserTags:
-                description: ExperimentalPropagateUserTags is an experimental
-                            flag that directs in-cluster operators to include the specified
-                            user tags in the tags of the AWS resources that the operators
-                            create. The field is deprecated.
-                type: boolean
                 propagateUserTags:
                 description: PropagateUserTags is a flag that directs in-cluster operators to include the specified
                             user tags in the tags of the AWS resources that the operators create.
                 type: boolean
 ```
+### Implementation constraints
+- The NLB is not reconciled with tag updates from service annotations by cloud-provider-aws controller. The [possible mitigations](https://github.com/openshift/cluster-ingress-operator/pull/1148#issuecomment-2423161470) are in  discussions by stakeholders on possible actions.
+- The AWS load balancer controller implementation considers tag information arguments as single source of truth. Hence, removes any differing tag keys set on the load balancer. As a mitigation, it is suggested for user to set the tags on service annotations rather than AWS resource directly.
 
 ### Risks and Mitigations
 
@@ -157,7 +154,6 @@ This enhancement updates `experimentalPropagateUserTags` field.
 On upgrade:
 
 - The new status field won't be populated since it is only populated by the installer and that can't have happened if the cluster was installed from a prior version. Components that consume the new field should take no action since they will see no additional tags.
-- The `experimentalPropagateUserTags` field will be deprecated in the GA release version to support updates to existing usages in scripts or configs and will be removed in the version after the GA release version.
 
 On downgrade:
 
@@ -552,3 +548,5 @@ As AWS considers empty string as a valid tag value, the behaviour becomes ambigu
 
 ## Infrastructure Needed [optional]
 
+
+