[Bug] On openshift with nat, all machines connect registry server with a fixed ip 10.129.0.6

[yalzhang]

Detial steps refer to:
#146 (comment)
We may need a new machine identifier.

[Jakob-Naucke]

Ah yes. So this could be patched easily by dropping this:

operator/register-server/src/main.rs

Lines 151 to 159 in 95b2c71

	for existing_machine in machine_list.items {
	if existing_machine.spec.registration_address == client_ip {
	if let Some(name) = &existing_machine.metadata.name {
	info!("Found existing machine {name} with IP {client_ip}, deleting...");
	machines.delete(name, &Default::default()).await?;
	info!("Deleted existing machine: {name}");
	}
	}
	}

and I'd like to ask @alicefr what we really gain from having that

[alicefr]

Ah yes. So this could be patched easily by dropping this:

operator/register-server/src/main.rs

Lines 151 to 159 in 95b2c71

for existing_machine in machine_list.items {
if existing_machine.spec.registration_address == client_ip {
if let Some(name) = &existing_machine.metadata.name {
info!("Found existing machine {name} with IP {client_ip}, deleting...");
machines.delete(name, &Default::default()).await?;
info!("Deleted existing machine: {name}");
}
}
}
and I'd like to ask @alicefr what we really gain from having that

The problem is that if the machine does multiple request, it creates multiple machine object. Ideally, we want to have a unique object per machine. For example, you can now use the machine as parent object for the AK and secrets and they get garbage collected when the machine is deleted. The problem is that a part from the ip there isn't another way to identify the machine. Maybe we should start using the mac address instead of the IP. WDYT?

[Jakob-Naucke]

MAC isn't transmitted. You could include an idempotency key in the request.

[alicefr]

MAC isn't transmitted. You could include an idempotency key in the request.

What if the the machine fails a couple of times the registration? Would you write this key in var?

UPDATE: it is equivalent of let the machine generate the uuid, instead of our operator

[Jakob-Naucke]

it is equivalent of let the machine generate the uuid, instead of our operator

Yes.

[alicefr]

it is equivalent of let the machine generate the uuid, instead of our operator

Yes.

we need to update ignition for this. I can start modifing it tomorrow

[alicefr]

@Jakob-Naucke it isn't possible to generate the uuid on the VM side, since the registration is done with the ignition fetch merge directive. It is a standard API, I don't think they will accept that we add the uuid to that

UPDATE: the information to distinguish every machine needs to be in the http request

[alicefr]

Otherwise we can use a dynamic url for ignition where we append the uuid, however, it needs to be configured by MCO or some other tools that configure ignition for the machine. We are back to the problem that the initial ignition needs to be the same for every machine

[alicefr]

@yalzhang are you using multiple machine on the same host?

[alicefr]

The other option is that we relax this check and that if a machine does a successful registration, it won't re do it again and the machine are unique. The worst scenario is that we could pollute the cluster with a lot of machine objects. Might be a good DOS attack

[travier]

operator/register-server/src/main.rs

Lines 151 to 159 in 95b2c71

for existing_machine in machine_list.items {
if existing_machine.spec.registration_address == client_ip {
if let Some(name) = &existing_machine.metadata.name {
info!("Found existing machine {name} with IP {client_ip}, deleting...");
machines.delete(name, &Default::default()).await?;
info!("Deleted existing machine: {name}");
}
}
}

I would drop that as well. We don't want a CRD to be deleted if someone can fake an IP.

Let's drop all usage of IP addresses from the code base now that we create unique IDs during the registration process.

[travier]

Ideally we never delete any Machine CRD for now. Only manually. We'll do that only once we can match them with the actual machine in CAPI/MAPI.

[alicefr]

@travier @Jakob-Naucke Not strictly related to this issue, but we also use the IP in order to match the AK regstration key with the Machine object. This also has a similar problem, but in that case isn't a security mechanism but a core logic to match AK and the machine :/. Additionally, the AK registration runs before fetching of ignition. Hence, if we generate the ID at that point, it is too late for the AK registration. I'm afraid the ID needs to be generated by the machine during the AK registration, but this is problematic with Azure which doesn't require the AK registration.