From 5385eb0fb3afc564cc1e5f89469ea30d0171925b Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Mon, 13 Nov 2023 11:58:05 -0500 Subject: [PATCH] Add validation when setting peers - cannot peer to self - cannot peer to instance that is already peered to self Other changes: - ReceptorAddress protocol field restricted to choices: tcp, ws, wss - fix awx-manage list_instances when instance.last_seen is None - InstanceLink make source and target unique together - Add help text to the ReceptorAddress fields Signed-off-by: Seth Foster --- awx/api/serializers.py | 22 ++++++++++--- .../management/commands/list_instances.py | 2 +- awx/main/migrations/0188_inbound_hop_nodes.py | 31 ++++++++++++++----- awx/main/models/ha.py | 2 ++ awx/main/models/receptor_address.py | 20 ++++++++---- 5 files changed, 59 insertions(+), 18 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 34abf4ac42..9df4e0edf4 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -5505,17 +5505,17 @@ class ReceptorAddressSerializer(BaseSerializer): # only allow websocket_path to be set if protocol is ws if attrs.get('protocol') != 'ws' and attrs.get('websocket_path'): - raise serializers.ValidationError(_("Can only set websocket path if protocol is ws")) + raise serializers.ValidationError(_("Can only set websocket path if protocol is ws.")) # an instance can only have one address with peers_from_control_nodes set to True if peers_from_control_nodes: for other_address in ReceptorAddress.objects.filter(instance=instance.id): if other_address.peers_from_control_nodes: - raise serializers.ValidationError(_("Only one address can set peers_from_control_nodes to True")) + raise serializers.ValidationError(_("Only one address can set peers_from_control_nodes to True.")) # is_internal should be False if attrs.get('is_internal') == True: - raise serializers.ValidationError(_("Only external addresses can be created")) + raise serializers.ValidationError(_("Only external addresses can be created.")) return super().validate(attrs) @@ -5528,7 +5528,9 @@ class InstanceSerializer(BaseSerializer): jobs_running = serializers.IntegerField(help_text=_('Count of jobs in the running or waiting state that are targeted for this instance'), read_only=True) jobs_total = serializers.IntegerField(help_text=_('Count of all jobs that target this instance'), read_only=True) health_check_pending = serializers.SerializerMethodField() - peers = serializers.PrimaryKeyRelatedField(many=True, required=False, queryset=ReceptorAddress.objects.all()) + peers = serializers.PrimaryKeyRelatedField( + help_text=_('Primary keys of receptor addresses to peer to.'), many=True, required=False, queryset=ReceptorAddress.objects.all() + ) class Meta: model = Instance @@ -5645,6 +5647,18 @@ class InstanceSerializer(BaseSerializer): if check_peers_changed(): raise serializers.ValidationError(_("Cannot change peers.")) + # cannot peer to self + peers_ids = [p.id for p in attrs.get('peers', [])] + if self.instance and self.instance.receptor_addresses.filter(id__in=peers_ids).exists(): + raise serializers.ValidationError(_("Instance cannot peer to its own address.")) + + # cannot peer to an instance that is already peered to this instance + if self.instance and self.instance.receptor_addresses.all().exists(): + instance_addresses = set(self.instance.receptor_addresses.all()) + for p in attrs.get('peers', []): + if set(p.instance.peers.all()) & instance_addresses: + raise serializers.ValidationError(_(f"Instance {p.instance.hostname} is already peered to this instance.")) + return super().validate(attrs) def validate_node_type(self, value): diff --git a/awx/main/management/commands/list_instances.py b/awx/main/management/commands/list_instances.py index 4ebb6505e9..b2bbcfea29 100644 --- a/awx/main/management/commands/list_instances.py +++ b/awx/main/management/commands/list_instances.py @@ -55,7 +55,7 @@ class Command(BaseCommand): capacity = f' capacity={x.capacity}' if x.node_type != 'hop' else '' version = f" version={x.version or '?'}" if x.node_type != 'hop' else '' - heartbeat = f' heartbeat="{x.last_seen:%Y-%m-%d %H:%M:%S}"' if x.last_seen and x.capacity or x.node_type == 'hop' else '' + heartbeat = f' heartbeat="{x.last_seen:%Y-%m-%d %H:%M:%S}"' if x.last_seen else '' print(f'\t{color}{x.hostname}{capacity} node_type={x.node_type}{version}{heartbeat}{end_color}') print() diff --git a/awx/main/migrations/0188_inbound_hop_nodes.py b/awx/main/migrations/0188_inbound_hop_nodes.py index 963eb10e19..18d0a58681 100644 --- a/awx/main/migrations/0188_inbound_hop_nodes.py +++ b/awx/main/migrations/0188_inbound_hop_nodes.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.6 on 2023-11-09 19:11 +# Generated by Django 4.2.6 on 2023-11-13 16:10 import django.core.validators from django.db import migrations, models @@ -15,17 +15,30 @@ class Migration(migrations.Migration): name='ReceptorAddress', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('address', models.CharField(max_length=255)), + ('address', models.CharField(help_text='Routable address for this instance.', max_length=255)), ( 'port', models.IntegerField( - default=27199, validators=[django.core.validators.MinValueValidator(0), django.core.validators.MaxValueValidator(65535)] + default=27199, + help_text='Port for the address.', + validators=[django.core.validators.MinValueValidator(0), django.core.validators.MaxValueValidator(65535)], ), ), - ('protocol', models.CharField(default='tcp', max_length=10)), - ('websocket_path', models.CharField(blank=True, default='', max_length=255)), - ('is_internal', models.BooleanField(default=False)), - ('peers_from_control_nodes', models.BooleanField(default=False)), + ( + 'protocol', + models.CharField( + choices=[('tcp', 'TCP'), ('ws', 'WS'), ('wss', 'WSS')], + default='tcp', + help_text="Protocol to use when connecting, 'tcp' or 'ws'.", + max_length=10, + ), + ), + ('websocket_path', models.CharField(blank=True, default='', help_text='Websocket path.', max_length=255)), + ('is_internal', models.BooleanField(default=False, help_text='If True, only routable inside of the Kubernetes cluster.')), + ( + 'peers_from_control_nodes', + models.BooleanField(default=False, help_text='If True, control plane cluster nodes should automatically peer to it.'), + ), ], ), migrations.RemoveConstraint( @@ -45,6 +58,10 @@ class Migration(migrations.Migration): name='source', field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='main.instance'), ), + migrations.AddConstraint( + model_name='instancelink', + constraint=models.UniqueConstraint(fields=('source', 'target'), name='source_target_unique_together'), + ), migrations.AddField( model_name='receptoraddress', name='instance', diff --git a/awx/main/models/ha.py b/awx/main/models/ha.py index 3073466c7e..a5cf2846ca 100644 --- a/awx/main/models/ha.py +++ b/awx/main/models/ha.py @@ -6,6 +6,7 @@ import logging import os from django.core.validators import MinValueValidator, MaxValueValidator +from django.core.exceptions import ValidationError from django.db import models, connection from django.db.models.signals import post_save, post_delete from django.dispatch import receiver @@ -67,6 +68,7 @@ class HasPolicyEditsMixin(HasEditsMixin): class InstanceLink(BaseModel): class Meta: ordering = ("id",) + constraints = [models.UniqueConstraint(fields=['source', 'target'], name='source_target_unique_together')] source = models.ForeignKey('Instance', on_delete=models.CASCADE) target = models.ForeignKey('ReceptorAddress', on_delete=models.CASCADE) diff --git a/awx/main/models/receptor_address.py b/awx/main/models/receptor_address.py index 9599d0e9d5..b5101287ae 100644 --- a/awx/main/models/receptor_address.py +++ b/awx/main/models/receptor_address.py @@ -18,18 +18,26 @@ class ReceptorAddress(models.Model): ) ] - address = models.CharField(max_length=255) - port = models.IntegerField(default=27199, validators=[MinValueValidator(0), MaxValueValidator(65535)]) - protocol = models.CharField(max_length=10, default="tcp") - websocket_path = models.CharField(max_length=255, default="", blank=True) - is_internal = models.BooleanField(default=False) - peers_from_control_nodes = models.BooleanField(default=False) + class Protocols(models.TextChoices): + TCP = 'tcp', _('TCP') + WS = 'ws', _('WS') + WSS = 'wss', _('WSS') + + address = models.CharField(help_text=_("Routable address for this instance."), max_length=255) + port = models.IntegerField(help_text=_("Port for the address."), default=27199, validators=[MinValueValidator(0), MaxValueValidator(65535)]) + protocol = models.CharField(help_text=_("Protocol to use when connecting, 'tcp' or 'ws'."), max_length=10, default=Protocols.TCP, choices=Protocols.choices) + websocket_path = models.CharField(help_text=_("Websocket path."), max_length=255, default="", blank=True) + is_internal = models.BooleanField(help_text=_("If True, only routable inside of the Kubernetes cluster."), default=False) + peers_from_control_nodes = models.BooleanField(help_text=_("If True, control plane cluster nodes should automatically peer to it."), default=False) instance = models.ForeignKey( 'Instance', related_name='receptor_addresses', on_delete=models.CASCADE, ) + def __str__(self): + return self.get_full_address() + def get_full_address(self): scheme = "" path = ""