Skip to content

✨ external-resources: tagging support #5189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

✨ external-resources: tagging support #5189

wants to merge 1 commit into from

Conversation

chassing
Copy link
Member

@chassing chassing commented Aug 27, 2025 •
edited
Loading

Add tagging support to all ERv2-managed external resources:

The compliance mandatory ones:

  • app-code
  • cost-center
  • service-phase

And allow custom tags directly on the external resources via the new tag attribute.

Ticket: APPSRE-12423

Example dry-run output:

Terraform used the selected providers to generate the following execution                                                                                                                                           15:38:48 [42/9453]
plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:
  # aws_db_instance.this will be updated in-place
  ~ resource "aws_db_instance" "this" {
        id                                    = "db-XXXX"
      ~ tags                                  = {
            "app"                    = "notifications"
          + "app-code"               = "OSD-002"
            "cluster"                = "crcp01ue1"
          + "cost-center"            = "148"
            "environment"            = "insights-production"
            "managed_by_integration" = "external_resources"
            "namespace"              = "notifications-prod"
          + "service-phase"          = "prod"
        }
      ~ tags_all                              = {
          + "app-code"               = "OSD-002"
          + "cost-center"            = "148"
          + "service-phase"          = "prod"
            # (5 unchanged elements hidden)
        }
        # (60 unchanged attributes hidden)
        # (1 unchanged block hidden)
    }

  # aws_db_parameter_group.this["notifications-backend-prod-notifications-prod-postgres16-parameter-group"] will be updated in-place
  ~ resource "aws_db_parameter_group" "this" {
        id           = "notifications-backend-prod-notifications-prod-postgres16-parameter-group"
        name         = "notifications-backend-prod-notifications-prod-postgres16-parameter-group"
      ~ tags         = {
            "app"                    = "notifications"
          + "app-code"               = "OSD-002"
            "cluster"                = "crcp01ue1"
          + "cost-center"            = "148"
            "environment"            = "insights-production"
            "managed_by_integration" = "external_resources"
            "namespace"              = "notifications-prod"
          + "service-phase"          = "prod"
        }
      ~ tags_all     = {
          + "app-code"               = "OSD-002"
          + "cost-center"            = "148"
          + "service-phase"          = "prod"
            # (5 unchanged elements hidden)
        }
        # (5 unchanged attributes hidden)
        # (3 unchanged blocks hidden)
    }

  # aws_iam_role.this[0] will be updated in-place
  ~ resource "aws_iam_role" "this" {
        id                    = "notifications-backend-prod-enhanced-monitoring"
        name                  = "notifications-backend-prod-enhanced-monitoring"
      ~ tags                  = {
            "app"                    = "notifications"
          + "app-code"               = "OSD-002"
            "cluster"                = "crcp01ue1"
          + "cost-center"            = "148"
            "environment"            = "insights-production"
            "managed_by_integration" = "external_resources"
            "namespace"              = "notifications-prod"
          + "service-phase"          = "prod"
        }
      ~ tags_all              = {
          + "app-code"               = "OSD-002"
          + "cost-center"            = "148"
          + "service-phase"          = "prod"
            # (5 unchanged elements hidden)
        }
        # (8 unchanged attributes hidden)
    }

Plan: 0 to add, 3 to change, 0 to destroy.

@chassing chassing self-assigned this Aug 27, 2025
hemslo
hemslo reviewed Aug 28, 2025
View reviewed changes
reconcile/utils/external_resource_spec.py
Comment on lines +154 to +156
"app-code": self.namespace["app"].get("appCode", "unknown"),
"cost-center": self.namespace["app"].get("costCenter", "999"),
"service-phase": self.namespace["environment"].get("servicePhase", "prod"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why these default values? there is one hard coded default value

qontract-reconcile/reconcile/utils/terrascript_aws_client.py

Lines 289 to 293 in c048032

DEFAULT_TAGS = {
"tags": {
"app": "app-sre-infra",
},
}
, can we replace it with default tags in settings (need new schema), then pass down to terraform provider section to avoid missing tags?

reconcile/utils/external_resource_spec.py
Comment on lines +161 to +170
# normalize camelCase keys to kebab-case
key_mapping = {
"appCode": "app-code",
"costCenter": "cost-center",
"servicePhase": "service-phase",
}
normalized_tags = {}
for key, value in resource_tags.items():
normalized_key = key_mapping.get(key, key)
normalized_tags[normalized_key] = value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic maybe needed in default tags handling as well

reconcile/utils/external_resource_spec.py
@@ -144,13 +144,33 @@ def annotations(self) -> dict[str, str]:
return {}

def tags(self, integration: str) -> dict[str, str]:
return {
tags = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please update test cases in

qontract-reconcile/reconcile/test/utils/test_external_resource_spec.py

Line 201 in c048032

def test_spec_tags() -> None:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hemslo hemslo hemslo left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@chassing chassing

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chassing @hemslo