Skip to content

Commit 908bbb1

Browse files
authored
'tag_cluster' policy applying kubernetes.io/cluster/<name>=owned to Hypershift SG’s (#976)
* Initial code fix * Move constant to environment_variables.py * Reuse existing env variable * Review comments * Review comments * Add Unittests
1 parent ea338cf commit 908bbb1

2 files changed

Lines changed: 115 additions & 2 deletions

File tree

cloud_governance/policy/policy_operations/aws/tag_cluster/tag_cluster_resouces.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from cloud_governance.common.clouds.aws.utils.common_methods import get_tag_value_from_tags
44
from cloud_governance.common.logger.init_logger import logger
5+
from cloud_governance.main.environment_variables import environment_variables
56
from cloud_governance.policy.policy_operations.aws.tag_cluster.tag_cluster_operations import TagClusterOperations
67

78
from cloud_governance.policy.policy_operations.aws.tag_non_cluster.tag_non_cluster_resources import \
@@ -108,9 +109,15 @@ def __get_cluster_tags_by_instance_cluster(self, cluster_name: str):
108109
for tag in item.get('Tags'):
109110
if any(prefix in tag.get('Key', '') for prefix in self.cluster_prefix):
110111
if cluster_name in tag.get('Key'):
112+
# Propagation-blocked prefixes derived from CLUSTER_PREFIX (e.g. kubernetes.io/, sigs.k8s.io/)
113+
cluster_prefix = environment_variables.environment_variables_dict['CLUSTER_PREFIX']
114+
no_propagate_prefixes = tuple(p.split('/', 1)[0] + '/' for p in cluster_prefix)
111115
i_tags = [instance_tag for instance_tag in item.get('Tags') if
112-
instance_tag.get('Key') != 'Name']
113-
return [i_tag for i_tag in i_tags if i_tag.get('Key') != cluster_name]
116+
instance_tag.get('Key') != 'Name' and
117+
not any((instance_tag.get('Key') or '').startswith(prefix)
118+
for prefix in self.cluster_prefix) and
119+
not (instance_tag.get('Key') or '').startswith(no_propagate_prefixes)]
120+
return i_tags
114121
return []
115122

116123
def get_date_from_date(self, date_time: datetime):

tests/unittest/cloud_governance/aws/tag_cluster/test_tag_cluster_resources.py

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# TEST DRY RUN: mandatory_tags = None
22
import json
33
import os
4+
from unittest.mock import patch
45

56
import pytest
67
from moto import mock_ec2, mock_cloudtrail, mock_iam, mock_s3, mock_elb, mock_elbv2
@@ -236,3 +237,108 @@ def test_cluster_ec2():
236237
tag_resources = TagClusterResources(cluster_prefix=cluster_prefix, cluster_name=cluster_name,
237238
input_tags=mandatory_tags, region='us-east-2')
238239
assert len(tag_resources.cluster_instance()) == 3
240+
241+
242+
# --- Hypershift / tag_cluster propagation (PR #976): do not propagate kubernetes.io/ or sigs.k8s.io/ tags ---
243+
244+
CLUSTER_STAMP_KEY = 'kubernetes.io/cluster/hyper2-unittest'
245+
246+
247+
def test_get_cluster_tags_for_propagation_excludes_cluster_and_k8s_sigs_tags():
248+
"""
249+
Tags returned for propagation must not include kubernetes.io/cluster, other kubernetes.io/*,
250+
sigs.k8s.io/*, or Name — only governance-style tags (e.g. User) should propagate.
251+
"""
252+
with mock_ec2(), mock_iam(), mock_cloudtrail(), mock_s3(), mock_elb(), mock_elbv2():
253+
tcr = TagClusterResources(
254+
cluster_prefix=cluster_prefix,
255+
cluster_name=cluster_name,
256+
input_tags=mandatory_tags,
257+
region='us-east-2',
258+
)
259+
fake_instance = [{
260+
'InstanceId': 'i-hyper2test',
261+
'Tags': [
262+
{'Key': CLUSTER_STAMP_KEY, 'Value': 'owned'},
263+
{
264+
'Key': 'sigs.k8s.io/cluster-api-provider-aws/cluster/hyper2-unittest',
265+
'Value': 'owned',
266+
},
267+
{'Key': 'kubernetes.io/role/worker', 'Value': 'true'},
268+
{'Key': 'User', 'Value': 'alice'},
269+
{'Key': 'Manager', 'Value': 'bob'},
270+
{'Key': 'Name', 'Value': 'hypershift-node-1'},
271+
],
272+
}]
273+
with patch.object(tcr, '_get_instances_data', return_value=[fake_instance]):
274+
result = tcr._TagClusterResources__get_cluster_tags_by_instance_cluster(CLUSTER_STAMP_KEY)
275+
276+
assert result == [
277+
{'Key': 'User', 'Value': 'alice'},
278+
{'Key': 'Manager', 'Value': 'bob'},
279+
]
280+
assert not any(t['Key'].startswith('kubernetes.io/') for t in result)
281+
assert not any(t['Key'].startswith('sigs.k8s.io/') for t in result)
282+
assert not any(t['Key'] == 'Name' for t in result)
283+
284+
285+
def test_get_cluster_tags_for_propagation_empty_when_only_cluster_system_tags():
286+
"""If the instance has only cluster stamp and k8s/sigs system tags (plus Name), nothing should propagate."""
287+
with mock_ec2(), mock_iam(), mock_cloudtrail(), mock_s3(), mock_elb(), mock_elbv2():
288+
tcr = TagClusterResources(
289+
cluster_prefix=cluster_prefix,
290+
cluster_name=cluster_name,
291+
input_tags=mandatory_tags,
292+
region='us-east-2',
293+
)
294+
fake_instance = [{
295+
'InstanceId': 'i-onlysys',
296+
'Tags': [
297+
{'Key': CLUSTER_STAMP_KEY, 'Value': 'owned'},
298+
{
299+
'Key': 'sigs.k8s.io/cluster-api-provider-aws/cluster/hyper2-unittest',
300+
'Value': 'owned',
301+
},
302+
{'Key': 'kubernetes.io/role/master', 'Value': 'true'},
303+
{'Key': 'Name', 'Value': 'cp-0'},
304+
],
305+
}]
306+
with patch.object(tcr, '_get_instances_data', return_value=[fake_instance]):
307+
result = tcr._TagClusterResources__get_cluster_tags_by_instance_cluster(CLUSTER_STAMP_KEY)
308+
309+
assert result == []
310+
311+
312+
def test_get_cluster_tags_for_propagation_uses_cluster_prefix_from_environment_variables():
313+
"""
314+
no_propagate_prefixes must follow CLUSTER_PREFIX in environment_variables (same derivation as production).
315+
"""
316+
from cloud_governance.main.environment_variables import environment_variables
317+
318+
custom_prefixes = ['api.openshift.com/cluster', 'kubernetes.io/cluster']
319+
original = environment_variables.environment_variables_dict['CLUSTER_PREFIX']
320+
try:
321+
environment_variables.environment_variables_dict['CLUSTER_PREFIX'] = custom_prefixes
322+
with mock_ec2(), mock_iam(), mock_cloudtrail(), mock_s3(), mock_elb(), mock_elbv2():
323+
tcr = TagClusterResources(
324+
cluster_prefix=custom_prefixes,
325+
cluster_name=cluster_name,
326+
input_tags=mandatory_tags,
327+
region='us-east-2',
328+
)
329+
stamp = 'api.openshift.com/cluster/hyper2-unittest'
330+
fake_instance = [{
331+
'InstanceId': 'i-custom',
332+
'Tags': [
333+
{'Key': stamp, 'Value': 'owned'},
334+
{'Key': 'api.openshift.com/something-else', 'Value': 'x'},
335+
{'Key': 'User', 'Value': 'carol'},
336+
],
337+
}]
338+
with patch.object(tcr, '_get_instances_data', return_value=[fake_instance]):
339+
result = tcr._TagClusterResources__get_cluster_tags_by_instance_cluster(stamp)
340+
# api.openshift.com/ and kubernetes.io/ blocked; User kept
341+
assert result == [{'Key': 'User', 'Value': 'carol'}]
342+
assert not any(t['Key'].startswith('api.openshift.com/') for t in result)
343+
finally:
344+
environment_variables.environment_variables_dict['CLUSTER_PREFIX'] = original

0 commit comments

Comments
 (0)