From 9bb158cf794742d55579365b02b2ab6d63b7f038 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 21 Nov 2023 13:04:08 +0000 Subject: [PATCH 01/27] Base compatability with new OPTE ioctl API --- Cargo.lock | 223 ++++++++++++++++--------- Cargo.toml | 4 +- illumos-utils/src/opte/port_manager.rs | 13 +- 3 files changed, 152 insertions(+), 88 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 07f804b03d2..3c851e093bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -33,7 +33,7 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac1f845298e95f983ff1944b728ae08b8cebab80d684f0a832ed0fc74dfa27e2" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "cipher", "cpufeatures", ] @@ -58,7 +58,7 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2c99f64d1e06488f620f932677e24bc6e2897582980441ae90a671415bd7ec2f" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "once_cell", "version_check", ] @@ -381,7 +381,7 @@ checksum = "2089b7e3f35b9dd2d0ed921ead4f6d318c27680d4a5bd167b3ee120edb105837" dependencies = [ "addr2line", "cc", - "cfg-if 1.0.0", + "cfg-if", "libc", "miniz_oxide", "object 0.32.1", @@ -847,12 +847,6 @@ dependencies = [ "nom", ] -[[package]] -name = "cfg-if" -version = "0.1.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" - [[package]] name = "cfg-if" version = "1.0.0" @@ -865,7 +859,7 @@ version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c3613f74bd2eac03dad61bd53dbe620703d4371614fe0bc3b9f04dd36fe4e818" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "cipher", "cpufeatures", ] @@ -1003,6 +997,12 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cd7cc57abe963c6d3b9d8be5b06ba7c8957a930305ca90304f24ef040aa6f961" +[[package]] +name = "cobs" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67ba02a97a2bd10f4b59b25c7973101c79642302776489e030cd13cdab09ed15" + [[package]] name = "colorchoice" version = "1.0.0" @@ -1125,7 +1125,7 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b540bd8bc810d3885c6ea91e2018302f68baba2129ab3e88f32389ee9370880d" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", ] [[package]] @@ -1190,7 +1190,7 @@ version = "0.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2801af0d36612ae591caa9568261fddce32ce6e08a7275ea334a06a4ad021a2c" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "crossbeam-channel", "crossbeam-deque", "crossbeam-epoch", @@ -1204,7 +1204,7 @@ version = "0.5.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a33c2bf77f2df06183c3aa30d1e96c0695a313d4f9c453cc3762a6db39f99200" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "crossbeam-utils", ] @@ -1214,7 +1214,7 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ce6fd6f855243022dcecf8702fef0c297d4338e226845fe067f6341ad9fa0cef" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "crossbeam-epoch", "crossbeam-utils", ] @@ -1226,7 +1226,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ae211234986c545741a7dc064309f67ee1e5ad243d0e48335adc0484d960bcc7" dependencies = [ "autocfg", - "cfg-if 1.0.0", + "cfg-if", "crossbeam-utils", "memoffset 0.9.0", "scopeguard", @@ -1238,7 +1238,7 @@ version = "0.3.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d1cfb3ea8a53f37c40dea2c7bedcbd88bdfae54f5e2175d6ecaff1c988353add" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "crossbeam-utils", ] @@ -1248,7 +1248,7 @@ version = "0.8.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5a22b2d63d4d1dc0b7f1b6b2747dd0088008a9be28b6ddf0b1e7d335e3037294" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", ] [[package]] @@ -1364,7 +1364,7 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b6bd9c8e659a473bce955ae5c35b116af38af11a7acb0b480e01f3ed348aeb40" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "memchr", ] @@ -1383,7 +1383,7 @@ version = "4.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "622178105f911d937a42cdb140730ba4a3ed2becd8ae6ce39c7d28b5d75d4588" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "cpufeatures", "curve25519-dalek-derive", "digest", @@ -1482,7 +1482,7 @@ version = "5.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "edd72493923899c6f10c641bdbdeddc7183d6396641d99c1a0d1597f37f92e28" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "hashbrown 0.14.2", "lock_api", "once_cell", @@ -1550,6 +1550,38 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ffe7ed1d93f4553003e20b629abe9085e1e81b1429520f897f8f8860bc6dfc21" +[[package]] +name = "defmt" +version = "0.3.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8a2d011b2fee29fb7d659b83c43fce9a2cb4df453e16d441a51448e448f3f98" +dependencies = [ + "bitflags 1.3.2", + "defmt-macros", +] + +[[package]] +name = "defmt-macros" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "54f0216f6c5acb5ae1a47050a6645024e6edafc2ee32d421955eccfef12ef92e" +dependencies = [ + "defmt-parser", + "proc-macro-error", + "proc-macro2", + "quote", + "syn 2.0.32", +] + +[[package]] +name = "defmt-parser" +version = "0.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "269924c02afd7f94bc4cecbfa5c379f6ffcf9766b3408fe63d22c728654eccd0" +dependencies = [ + "thiserror", +] + [[package]] name = "der" version = "0.7.8" @@ -1729,7 +1761,7 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b98cf8ebf19c3d1b223e151f99a4f9f0690dca41414773390fc824184ac833e1" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "dirs-sys-next", ] @@ -2007,6 +2039,12 @@ dependencies = [ "zeroize", ] +[[package]] +name = "embedded-io" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ef1a6892d9eef45c8fa6b9e0086428a2cca8491aca8f787c534a3d6d0bcb3ced" + [[package]] name = "ena" version = "0.14.2" @@ -2028,7 +2066,7 @@ version = "0.8.33" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7268b386296a025e474d5140678f75d6de9493ae55a5d709eeb9dd08149945e1" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", ] [[package]] @@ -2150,7 +2188,7 @@ version = "3.0.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ef033ed5e9bad94e55838ca0ca906db0e043f517adda0c8b79c7a8c66c93c1b5" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "rustix 0.38.9", "windows-sys 0.48.0", ] @@ -2177,7 +2215,7 @@ version = "0.2.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d4029edd3e734da6fe05b6cd7bd2960760a616bd2ddd0d59a0124746d6272af0" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "libc", "redox_syscall 0.3.5", "windows-sys 0.48.0", @@ -2554,7 +2592,7 @@ version = "0.1.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8fc3cb4d91f53b50155bdcfd23f6a4c39ae1969c2ae85982b135750cccaf5fce" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "libc", "wasi 0.9.0+wasi-snapshot-preview1", ] @@ -2565,7 +2603,7 @@ version = "0.2.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "be4136b2a15dd319360be1c07d9933517ccf0be8f16bf62a3bee4f0d618df427" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "js-sys", "libc", "wasi 0.11.0+wasi-snapshot-preview1", @@ -3061,7 +3099,7 @@ dependencies = [ [[package]] name = "illumos-sys-hdrs" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=258a8b59902dd36fc7ee5425e6b1fb5fc80d4649#258a8b59902dd36fc7ee5425e6b1fb5fc80d4649" +source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" [[package]] name = "illumos-utils" @@ -3072,7 +3110,7 @@ dependencies = [ "bhyve_api", "byteorder", "camino", - "cfg-if 1.0.0", + "cfg-if", "crucible-smf", "futures", "ipnetwork", @@ -3275,7 +3313,7 @@ version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7a5bbe824c507c5da5956355e86a746d82e0e1464f65d862cc5e71da70e94b2c" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", ] [[package]] @@ -3463,10 +3501,10 @@ dependencies = [ [[package]] name = "kstat-macro" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=258a8b59902dd36fc7ee5425e6b1fb5fc80d4649#258a8b59902dd36fc7ee5425e6b1fb5fc80d4649" +source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" dependencies = [ "quote", - "syn 1.0.109", + "syn 2.0.32", ] [[package]] @@ -3556,7 +3594,7 @@ version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b67380fd3b2fbe7527a606e18729d21c6f3951633d0500574c4dc22d2d638b9f" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "winapi", ] @@ -3572,7 +3610,7 @@ version = "0.1.0" source = "git+https://github.com/oxidecomputer/netadm-sys#f114bd0d543d886cd453932e9f0967de57289bc2" dependencies = [ "anyhow", - "cfg-if 1.0.0", + "cfg-if", "colored", "dlpi", "libc", @@ -3851,7 +3889,7 @@ version = "0.11.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4c84490118f2ee2d74570d114f3d0493cbf02790df303d2707606c3e14e07c96" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "downcast", "fragile", "lazy_static", @@ -3866,7 +3904,7 @@ version = "0.11.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "22ce75669015c4f47b289fd4d4f56e894e4c96003ffdf3ac51313126f94c6cbb" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "proc-macro2", "quote", "syn 1.0.109", @@ -4197,7 +4235,7 @@ version = "0.26.2" source = "git+https://github.com/jgallagher/nix?branch=r0.26-illumos#c1a3636db0524f194b714cfd117cd9b637b8b10e" dependencies = [ "bitflags 1.3.2", - "cfg-if 1.0.0", + "cfg-if", "libc", "memoffset 0.7.1", "pin-utils", @@ -4805,7 +4843,7 @@ dependencies = [ "camino", "camino-tempfile", "cancel-safe-futures", - "cfg-if 1.0.0", + "cfg-if", "chrono", "clap 4.4.3", "crucible-agent-client", @@ -5100,7 +5138,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bac25ee399abb46215765b1cb35bc0212377e58a061560d8b29b024fd0430e7c" dependencies = [ "bitflags 2.4.0", - "cfg-if 1.0.0", + "cfg-if", "foreign-types 0.3.2", "libc", "once_cell", @@ -5140,37 +5178,38 @@ dependencies = [ [[package]] name = "opte" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=258a8b59902dd36fc7ee5425e6b1fb5fc80d4649#258a8b59902dd36fc7ee5425e6b1fb5fc80d4649" +source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" dependencies = [ - "cfg-if 0.1.10", + "cfg-if", + "crc32fast", "dyn-clone", + "heapless", "illumos-sys-hdrs", "kstat-macro", "opte-api", "postcard", "serde", - "smoltcp 0.8.2", + "smoltcp 0.10.0", "version_check", - "zerocopy 0.6.4", + "zerocopy 0.7.26", ] [[package]] name = "opte-api" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=258a8b59902dd36fc7ee5425e6b1fb5fc80d4649#258a8b59902dd36fc7ee5425e6b1fb5fc80d4649" +source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" dependencies = [ - "cfg-if 0.1.10", "illumos-sys-hdrs", "ipnetwork", "postcard", "serde", - "smoltcp 0.8.2", + "smoltcp 0.10.0", ] [[package]] name = "opte-ioctl" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=258a8b59902dd36fc7ee5425e6b1fb5fc80d4649#258a8b59902dd36fc7ee5425e6b1fb5fc80d4649" +source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" dependencies = [ "libc", "libnet", @@ -5244,14 +5283,13 @@ dependencies = [ [[package]] name = "oxide-vpc" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=258a8b59902dd36fc7ee5425e6b1fb5fc80d4649#258a8b59902dd36fc7ee5425e6b1fb5fc80d4649" +source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" dependencies = [ - "cfg-if 0.1.10", "illumos-sys-hdrs", "opte", "serde", - "smoltcp 0.8.2", - "zerocopy 0.6.4", + "smoltcp 0.10.0", + "zerocopy 0.7.26", ] [[package]] @@ -5369,7 +5407,7 @@ dependencies = [ name = "oximeter-instruments" version = "0.1.0" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "chrono", "dropshot", "futures", @@ -5477,7 +5515,7 @@ version = "0.8.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "60a2cfe6f0ad2bfc16aefa463b497d5c7a5ecd44a23efa72aa342d90177356dc" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "instant", "libc", "redox_syscall 0.2.16", @@ -5491,7 +5529,7 @@ version = "0.9.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "93f00c865fe7cabf650081affecd3871070f26767e7b2070a3ffae14c654b447" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "libc", "redox_syscall 0.3.5", "smallvec 1.11.0", @@ -5840,7 +5878,7 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d52cff9d1d4dee5fe6d03729099f4a310a41179e0a10dbf542039873f2e826fb" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "cpufeatures", "opaque-debug", "universal-hash", @@ -5854,20 +5892,15 @@ checksum = "31114a898e107c51bb1609ffaf55a0e011cf6a4d7f1170d0015a165082c0338b" [[package]] name = "postcard" -version = "0.7.3" +version = "1.0.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a25c0b0ae06fcffe600ad392aabfa535696c8973f2253d9ac83171924c58a858" +checksum = "a55c51ee6c0db07e68448e336cf8ea4131a620edefebf9893e759b2d793420f8" dependencies = [ - "postcard-cobs", + "cobs", + "embedded-io", "serde", ] -[[package]] -name = "postcard-cobs" -version = "0.1.5-pre" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7c68cb38ed13fd7bc9dd5db8f165b7c8d9c1a315104083a2b10f11354c2af97f" - [[package]] name = "postgres-protocol" version = "0.6.6" @@ -6699,7 +6732,7 @@ version = "0.18.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d428f8247852f894ee1be110b375111b586d4fa431f6c46e64ba5a0dcccbe605" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "glob", "proc-macro2", "quote", @@ -7366,7 +7399,7 @@ version = "0.10.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e3bf829a2d51ab4a5ddf1352d8470c140cadc8301b2ae1789db023f01cedd6ba" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "cpufeatures", "digest", ] @@ -7377,7 +7410,7 @@ version = "0.10.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "793db75ad2bcafc3ffa7c68b215fee268f537982cd901d132f89c6343f3a3dc8" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "cpufeatures", "digest", ] @@ -7537,7 +7570,7 @@ version = "0.1.0" dependencies = [ "anyhow", "camino", - "cfg-if 1.0.0", + "cfg-if", "futures", "illumos-devinfo", "illumos-utils", @@ -7565,7 +7598,7 @@ dependencies = [ "async-trait", "camino", "camino-tempfile", - "cfg-if 1.0.0", + "cfg-if", "derive_more", "glob", "illumos-utils", @@ -7722,24 +7755,27 @@ dependencies = [ [[package]] name = "smoltcp" -version = "0.8.2" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee34c1e1bfc7e9206cc0fb8030a90129b4e319ab53856249bb27642cab914fb3" +checksum = "7e9786ac45091b96f946693e05bfa4d8ca93e2d3341237d97a380107a6b38dea" dependencies = [ "bitflags 1.3.2", "byteorder", + "cfg-if", + "heapless", "managed", ] [[package]] name = "smoltcp" -version = "0.9.1" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7e9786ac45091b96f946693e05bfa4d8ca93e2d3341237d97a380107a6b38dea" +checksum = "8d2e3a36ac8fea7b94e666dfa3871063d6e0a5c9d5d4fec9a1a6b7b6760f0229" dependencies = [ "bitflags 1.3.2", "byteorder", - "cfg-if 1.0.0", + "cfg-if", + "defmt", "heapless", "managed", ] @@ -8174,7 +8210,7 @@ version = "3.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cb94d2f3cc536af71caac6b6fcebf65860b347e7ce0cc9ebe8f70d3e521054ef" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "fastrand", "redox_syscall 0.3.5", "rustix 0.38.9", @@ -8315,7 +8351,7 @@ version = "1.1.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3fdd6f064ccff2d6567adcb3873ca630700f00b5ad3f060c25b5dcfd9a4ce152" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "once_cell", ] @@ -8681,7 +8717,7 @@ version = "0.1.37" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ce8c33a8d48bd45d624a6e523445fd21ec13d3653cd51f681abf67418f54eb8" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "log", "pin-project-lite", "tracing-attributes", @@ -8714,7 +8750,7 @@ version = "0.22.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c408c32e6a9dbb38037cece35740f2cf23c875d8ca134d33631cec83f74d3fe" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "data-encoding", "futures-channel", "futures-util", @@ -8735,7 +8771,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4f7f83d1e4a0e4358ac54c5c3681e5d7da5efc5a7a632c90bb6d6669ddd9bc26" dependencies = [ "async-trait", - "cfg-if 1.0.0", + "cfg-if", "data-encoding", "enum-as-inner", "futures-channel", @@ -8759,7 +8795,7 @@ version = "0.22.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aff21aa4dcefb0a1afbfac26deb0adc93888c7d295fb63ab273ef276ba2b7cfe" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "futures-util", "ipconfig", "lazy_static", @@ -8781,7 +8817,7 @@ checksum = "99022f9befa6daec2a860be68ac28b1f0d9d7ccf441d8c5a695e35a58d88840d" dependencies = [ "async-trait", "bytes", - "cfg-if 1.0.0", + "cfg-if", "enum-as-inner", "futures-executor", "futures-util", @@ -9296,7 +9332,7 @@ version = "0.2.87" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7706a72ab36d8cb1f80ffbf0e071533974a60d0a308d01a5d0375bf60499a342" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "wasm-bindgen-macro", ] @@ -9321,7 +9357,7 @@ version = "0.4.37" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c02dbc21516f9f1f04f187958890d7e6026df8d16540b7ad9492bc34a67cea03" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "js-sys", "wasm-bindgen", "web-sys", @@ -9777,7 +9813,7 @@ version = "0.50.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "524e57b2c537c0f9b1e69f1965311ec12182b4122e45035b1508cd24d2adadb1" dependencies = [ - "cfg-if 1.0.0", + "cfg-if", "windows-sys 0.48.0", ] @@ -9858,6 +9894,16 @@ dependencies = [ "zerocopy-derive 0.6.4", ] +[[package]] +name = "zerocopy" +version = "0.7.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e97e415490559a91254a2979b4829267a57d2fcd741a98eee8b722fb57289aa0" +dependencies = [ + "byteorder", + "zerocopy-derive 0.7.26", +] + [[package]] name = "zerocopy-derive" version = "0.2.0" @@ -9880,6 +9926,17 @@ dependencies = [ "syn 2.0.32", ] +[[package]] +name = "zerocopy-derive" +version = "0.7.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dd7e48ccf166952882ca8bd778a43502c64f33bf94c12ebe2a7f08e5a0f6689f" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.32", +] + [[package]] name = "zeroize" version = "1.6.0" diff --git a/Cargo.toml b/Cargo.toml index e4588efbde0..8bf90c9d733 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -260,7 +260,7 @@ omicron-sled-agent = { path = "sled-agent" } omicron-test-utils = { path = "test-utils" } omicron-zone-package = "0.9.1" oxide-client = { path = "clients/oxide-client" } -oxide-vpc = { git = "https://github.com/oxidecomputer/opte", rev = "258a8b59902dd36fc7ee5425e6b1fb5fc80d4649", features = [ "api", "std" ] } +oxide-vpc = { git = "https://github.com/oxidecomputer/opte", rev = "8e1b01baf5c161017a966f139fb774eb8f32d130", features = [ "api", "std" ] } once_cell = "1.18.0" openapi-lint = { git = "https://github.com/oxidecomputer/openapi-lint", branch = "main" } openapiv3 = "1.0" @@ -268,7 +268,7 @@ openapiv3 = "1.0" openssl = "0.10" openssl-sys = "0.9" openssl-probe = "0.1.5" -opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "258a8b59902dd36fc7ee5425e6b1fb5fc80d4649" } +opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "8e1b01baf5c161017a966f139fb774eb8f32d130" } oso = "0.27" owo-colors = "3.5.0" oximeter = { path = "oximeter/oximeter" } diff --git a/illumos-utils/src/opte/port_manager.rs b/illumos-utils/src/opte/port_manager.rs index f0a8d8d8396..bf549bd4ffb 100644 --- a/illumos-utils/src/opte/port_manager.rs +++ b/illumos-utils/src/opte/port_manager.rs @@ -20,6 +20,7 @@ use omicron_common::api::internal::shared::NetworkInterfaceKind; use omicron_common::api::internal::shared::SourceNatConfig; use oxide_vpc::api::AddRouterEntryReq; use oxide_vpc::api::DhcpCfg; +use oxide_vpc::api::ExternalIpCfg; use oxide_vpc::api::IpCfg; use oxide_vpc::api::IpCidr; use oxide_vpc::api::Ipv4Cfg; @@ -116,6 +117,8 @@ impl PortManager { // that OPTE supports. The array is guaranteed to be limited by Nexus. // See https://github.com/oxidecomputer/omicron/issues/1467 // See https://github.com/oxidecomputer/opte/issues/196 + + // XXX: Need to take ephemeral vs floating as separate params. let external_ip = external_ips.get(0); macro_rules! ip_cfg { @@ -152,7 +155,7 @@ impl PortManager { } None => None, }; - let external_ip = match external_ip { + let ephemeral_ip = match external_ip { Some($ip_t(ip)) => Some((*ip).into()), Some(_) => { error!( @@ -169,8 +172,12 @@ impl PortManager { vpc_subnet, private_ip: $ip.into(), gateway_ip: gateway_ip.into(), - snat, - external_ips: external_ip, + external_ips: ExternalIpCfg { + ephemeral_ip, + snat, + // TODO: implement + floating_ips: vec![], + }, }) }} } From a1aa492761b5b845a0c5122d6bd43188c7305b99 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 22 Nov 2023 13:29:52 +0000 Subject: [PATCH 02/27] Initial implementation of add/list floating IP endpoint --- nexus/db-model/src/external_ip.rs | 58 +++++++++++++++---- nexus/db-model/src/schema.rs | 5 +- .../src/db/datastore/external_ip.rs | 37 ++++++++++++ nexus/db-queries/src/db/datastore/mod.rs | 15 +++-- nexus/db-queries/src/db/datastore/rack.rs | 16 ++--- .../db-queries/src/db/queries/external_ip.rs | 2 +- nexus/src/app/external_ip.rs | 51 ++++++++++++++++ nexus/src/external_api/http_entrypoints.rs | 51 ++++++++++++++++ nexus/types/src/external_api/params.rs | 16 +++++ schema/crdb/dbinit.sql | 27 ++++++++- 10 files changed, 248 insertions(+), 30 deletions(-) diff --git a/nexus/db-model/src/external_ip.rs b/nexus/db-model/src/external_ip.rs index 1152e0109c8..98960a6372f 100644 --- a/nexus/db-model/src/external_ip.rs +++ b/nexus/db-model/src/external_ip.rs @@ -57,8 +57,9 @@ pub struct ExternalIp { pub time_created: DateTime, pub time_modified: DateTime, pub time_deleted: Option>, - pub ip_pool_id: Uuid, - pub ip_pool_range_id: Uuid, + pub project_id: Option, + pub ip_pool_id: Option, + pub ip_pool_range_id: Option, pub is_service: bool, // This is Some(_) for: // - all instance/service SNAT IPs @@ -92,7 +93,8 @@ pub struct IncompleteExternalIp { kind: IpKind, is_service: bool, parent_id: Option, - pool_id: Uuid, + pool_id: Option, + project_id: Option, // Optional address requesting that a specific IP address be allocated. explicit_ip: Option, // Optional range when requesting a specific SNAT range be allocated. @@ -113,7 +115,8 @@ impl IncompleteExternalIp { kind: IpKind::SNat, is_service: false, parent_id: Some(instance_id), - pool_id, + pool_id: Some(pool_id), + project_id: None, explicit_ip: None, explicit_port_range: None, } @@ -128,7 +131,8 @@ impl IncompleteExternalIp { kind: IpKind::Ephemeral, is_service: false, parent_id: Some(instance_id), - pool_id, + pool_id: Some(pool_id), + project_id: None, explicit_ip: None, explicit_port_range: None, } @@ -138,6 +142,7 @@ impl IncompleteExternalIp { id: Uuid, name: &Name, description: &str, + project_id: Uuid, pool_id: Uuid, ) -> Self { Self { @@ -148,12 +153,35 @@ impl IncompleteExternalIp { kind: IpKind::Floating, is_service: false, parent_id: None, - pool_id, + pool_id: Some(pool_id), + project_id: Some(project_id), explicit_ip: None, explicit_port_range: None, } } + pub fn for_floating_explicit( + id: Uuid, + name: &Name, + description: &str, + project_id: Uuid, + explicit_ip: IpAddr, + ) -> Self { + Self { + id, + name: Some(name.clone()), + description: Some(description.to_string()), + time_created: Utc::now(), + kind: IpKind::Floating, + is_service: false, + parent_id: None, + pool_id: None, + project_id: Some(project_id), + explicit_ip: Some(explicit_ip.into()), + explicit_port_range: None, + } + } + pub fn for_service_explicit( id: Uuid, name: &Name, @@ -170,7 +198,8 @@ impl IncompleteExternalIp { kind: IpKind::Floating, is_service: true, parent_id: Some(service_id), - pool_id, + pool_id: Some(pool_id), + project_id: None, explicit_ip: Some(IpNetwork::from(address)), explicit_port_range: None, } @@ -198,7 +227,8 @@ impl IncompleteExternalIp { kind: IpKind::SNat, is_service: true, parent_id: Some(service_id), - pool_id, + pool_id: Some(pool_id), + project_id: None, explicit_ip: Some(IpNetwork::from(address)), explicit_port_range, } @@ -219,7 +249,8 @@ impl IncompleteExternalIp { kind: IpKind::Floating, is_service: true, parent_id: Some(service_id), - pool_id, + pool_id: Some(pool_id), + project_id: None, explicit_ip: None, explicit_port_range: None, } @@ -234,7 +265,8 @@ impl IncompleteExternalIp { kind: IpKind::SNat, is_service: true, parent_id: Some(service_id), - pool_id, + pool_id: Some(pool_id), + project_id: None, explicit_ip: None, explicit_port_range: None, } @@ -268,10 +300,14 @@ impl IncompleteExternalIp { &self.parent_id } - pub fn pool_id(&self) -> &Uuid { + pub fn pool_id(&self) -> &Option { &self.pool_id } + pub fn project_id(&self) -> &Option { + &self.project_id + } + pub fn explicit_ip(&self) -> &Option { &self.explicit_ip } diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index 7f7dd570279..f5fc2eb567d 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -524,8 +524,9 @@ table! { time_created -> Timestamptz, time_modified -> Timestamptz, time_deleted -> Nullable, - ip_pool_id -> Uuid, - ip_pool_range_id -> Uuid, + project_id -> Nullable, + ip_pool_id -> Nullable, + ip_pool_range_id -> Nullable, is_service -> Bool, parent_id -> Nullable, kind -> crate::IpKindEnum, diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index e663130a846..766c49e991e 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -126,6 +126,26 @@ impl DataStore { self.allocate_external_ip(opctx, data).await } + /// Allocates a floating IP address for instance usage. + pub async fn allocate_floating_ip( + &self, + opctx: &OpContext, + project_id: Uuid, + ip_id: Uuid, + name: &Name, + description: &str, + ip: IpAddr, + ) -> CreateResult { + let data = IncompleteExternalIp::for_floating_explicit( + ip_id, + name, + description, + project_id, + ip, + ); + self.allocate_external_ip(opctx, data).await + } + async fn allocate_external_ip( &self, opctx: &OpContext, @@ -281,4 +301,21 @@ impl DataStore { .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + + /// Fetch all floating IP addresses for the provided project. + pub async fn lookup_floating_ips( + &self, + opctx: &OpContext, + project_id: Uuid, + ) -> LookupResult> { + use db::schema::external_ip::dsl; + dsl::external_ip + .filter(dsl::project_id.eq(project_id)) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::kind.eq(IpKind::Floating)) + .select(ExternalIp::as_select()) + .get_results_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } } diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index 0612b960c92..22b23cbef31 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -1565,8 +1565,9 @@ mod test { time_created: now, time_modified: now, time_deleted: None, - ip_pool_id: Uuid::new_v4(), - ip_pool_range_id: Uuid::new_v4(), + ip_pool_id: Some(Uuid::new_v4()), + ip_pool_range_id: Some(Uuid::new_v4()), + project_id: None, is_service: false, parent_id: Some(instance_id), kind: IpKind::Ephemeral, @@ -1625,8 +1626,9 @@ mod test { time_created: now, time_modified: now, time_deleted: None, - ip_pool_id: Uuid::new_v4(), - ip_pool_range_id: Uuid::new_v4(), + ip_pool_id: Some(Uuid::new_v4()), + ip_pool_range_id: Some(Uuid::new_v4()), + project_id: None, is_service: false, parent_id: Some(Uuid::new_v4()), kind: IpKind::SNat, @@ -1695,8 +1697,9 @@ mod test { time_created: now, time_modified: now, time_deleted: None, - ip_pool_id: Uuid::new_v4(), - ip_pool_range_id: Uuid::new_v4(), + ip_pool_id: Some(Uuid::new_v4()), + ip_pool_range_id: Some(Uuid::new_v4()), + project_id: None, is_service: false, parent_id: Some(Uuid::new_v4()), kind: IpKind::Floating, diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index 2cc58804702..1c8f2f1f04e 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -1148,31 +1148,31 @@ mod test { assert_eq!(observed_ip_pool_ranges[0].ip_pool_id, svc_pool.id()); // Verify the allocated external IPs - assert_eq!(dns_external_ip.ip_pool_id, svc_pool.id()); + assert_eq!(dns_external_ip.ip_pool_id, Some(svc_pool.id())); assert_eq!( dns_external_ip.ip_pool_range_id, - observed_ip_pool_ranges[0].id + Some(observed_ip_pool_ranges[0].id) ); assert_eq!(dns_external_ip.ip.ip(), external_dns_ip); - assert_eq!(nexus_external_ip.ip_pool_id, svc_pool.id()); + assert_eq!(nexus_external_ip.ip_pool_id, Some(svc_pool.id())); assert_eq!( nexus_external_ip.ip_pool_range_id, - observed_ip_pool_ranges[0].id + Some(observed_ip_pool_ranges[0].id) ); assert_eq!(nexus_external_ip.ip.ip(), nexus_ip); - assert_eq!(ntp1_external_ip.ip_pool_id, svc_pool.id()); + assert_eq!(ntp1_external_ip.ip_pool_id, Some(svc_pool.id())); assert_eq!( ntp1_external_ip.ip_pool_range_id, - observed_ip_pool_ranges[0].id + Some(observed_ip_pool_ranges[0].id) ); assert_eq!(ntp1_external_ip.ip.ip(), ntp1_ip); - assert_eq!(ntp2_external_ip.ip_pool_id, svc_pool.id()); + assert_eq!(ntp2_external_ip.ip_pool_id, Some(svc_pool.id())); assert_eq!( ntp2_external_ip.ip_pool_range_id, - observed_ip_pool_ranges[0].id + Some(observed_ip_pool_ranges[0].id) ); assert_eq!(ntp2_external_ip.ip.ip(), ntp2_ip); diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index cf182e080d8..f93b26b9efb 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -622,7 +622,7 @@ impl NextExternalIp { out.push_sql(" WHERE "); out.push_identifier(dsl::ip_pool_id::NAME)?; out.push_sql(" = "); - out.push_bind_param::(self.ip.pool_id())?; + out.push_bind_param::, Option>(self.ip.pool_id())?; out.push_sql(" AND "); out.push_identifier(dsl::time_deleted::NAME)?; out.push_sql(" IS NULL"); diff --git a/nexus/src/app/external_ip.rs b/nexus/src/app/external_ip.rs index 2354e970858..f3d1cb16bc5 100644 --- a/nexus/src/app/external_ip.rs +++ b/nexus/src/app/external_ip.rs @@ -9,7 +9,11 @@ use nexus_db_queries::authz; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::lookup; use nexus_db_queries::db::model::IpKind; +use nexus_types::external_api::params; +use omicron_common::api::external::CreateResult; +use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; +use uuid::Uuid; impl super::Nexus { pub(crate) async fn instance_list_external_ips( @@ -33,4 +37,51 @@ impl super::Nexus { }) .collect::>()) } + + pub(crate) async fn list_floating_ips( + &self, + opctx: &OpContext, + project_lookup: &lookup::Project<'_>, + ) -> ListResultVec { + let (.., authz_project) = + project_lookup.lookup_for(authz::Action::Read).await?; + Ok(self + .db_datastore + .lookup_floating_ips(opctx, authz_project.id()) + .await? + .into_iter() + .map(|ip| { + ip.try_into().unwrap() + }) + .collect::>()) + } + + pub(crate) async fn create_floating_ip( + &self, + opctx: &OpContext, + project_lookup: &lookup::Project<'_>, + params: ¶ms::FloatingIpCreate, + ) -> CreateResult { + let (.., authz_project) = + project_lookup.lookup_for(authz::Action::CreateChild).await?; + + let chosen_addr = match (¶ms.pool, params.address) { + (Some(_), _) => todo!("Drawing floating IP from pools not yet supported."), + (None, Some(ip)) => ip, + _ => return Err(Error::invalid_request("floating IP needs a pool or ")), + }; + + Ok(self + .db_datastore + .allocate_floating_ip( + opctx, + authz_project.id(), + Uuid::new_v4(), + ¶ms.identity.name.clone().into(), + ¶ms.identity.description, + chosen_addr) + .await? + .try_into().unwrap() + ) + } } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 428632bcf5b..5e15ed964f2 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -139,6 +139,9 @@ pub(crate) fn external_api() -> NexusApiDescription { api.register(ip_pool_service_range_add)?; api.register(ip_pool_service_range_remove)?; + api.register(floating_ip_list)?; + api.register(floating_ip_create)?; + api.register(disk_list)?; api.register(disk_create)?; api.register(disk_view)?; @@ -1518,6 +1521,54 @@ async fn ip_pool_service_range_remove( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +// Floating IP Addresses + +/// List all Floating IPs +#[endpoint { + method = GET, + path = "/v1/system/floating-ips", + tags = ["system/networking"], +}] +async fn floating_ip_list( + rqctx: RequestContext>, + query_params: Query, +)-> Result>, HttpError> { + let apictx = rqctx.context(); + let handler = async { + let nexus = &apictx.nexus; + let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + let project_lookup = + nexus.project_lookup(&opctx, query_params.into_inner())?; + let ips = + nexus.list_floating_ips(&opctx, &project_lookup).await?; + Ok(HttpResponseOk(ResultsPage { items: ips, next_page: None })) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/// Create a Floating IP +#[endpoint { + method = POST, + path = "/v1/system/floating-ips", + tags = ["system/networking"], +}] +async fn floating_ip_create( + rqctx: RequestContext>, + query_params: Query, + floating_params: TypedBody, +) -> Result, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let floating_params = floating_params.into_inner(); + let handler = async { + let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + let project_lookup = nexus.project_lookup(&opctx, query_params.into_inner())?; + let pool = nexus.create_floating_ip(&opctx, &project_lookup, &floating_params).await?; + Ok(HttpResponseCreated(views::ExternalIp::from(pool))) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + // Disks /// List disks diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index a0169ae7774..1fa0e521798 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -751,6 +751,22 @@ pub struct IpPoolUpdate { pub identity: IdentityMetadataUpdateParams, } +// Floating IPs +/// Parameters for creating a new floating IP address for instances. +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct FloatingIpCreate { + #[serde(flatten)] + pub identity: IdentityMetadataCreateParams, + + /// The intended address + // TODO: make non-optional and draw from pool + pub address: Option, + + /// The parent pool that a + // TODO: support tie-in to pools. + pub pool: Option, +} + // INSTANCES /// Describes an attachment of an `InstanceNetworkInterface` to an `Instance`, diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index fc3bc37fd72..7d451dfd013 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -1579,10 +1579,13 @@ CREATE TABLE IF NOT EXISTS omicron.public.external_ip ( time_deleted TIMESTAMPTZ, /* FK to the `ip_pool` table. */ - ip_pool_id UUID NOT NULL, + project_id UUID, + + /* FK to the `ip_pool` table. */ + ip_pool_id UUID, /* FK to the `ip_pool_range` table. */ - ip_pool_range_id UUID NOT NULL, + ip_pool_range_id UUID, /* True if this IP is associated with a service rather than an instance. */ is_service BOOL NOT NULL, @@ -1614,6 +1617,26 @@ CREATE TABLE IF NOT EXISTS omicron.public.external_ip ( (kind = 'floating' AND description IS NOT NULL) ), + /* Only floating IPs can be attached to a project. + * Projects are nullable in such a case. + */ + CONSTRAINT null_project_id CHECK ( + kind = 'floating' OR project_id IS NULL + ), + + /* Ephemeral/SNAT IPs must have a parent pool and range, while this + * is optional for floating IPs. + */ + CONSTRAINT null_non_fip_pool_id CHECK ( + kind = 'floating' OR (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) + ), + + /* If the IP pool is defined, the IP pool range must also be. */ + CONSTRAINT null_pool_range_id CHECK ( + (ip_pool_id IS NULL AND ip_pool_range_id IS NULL) OR + (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) + ), + /* * Only nullable if this is a floating IP, which may exist not * attached to any instance or service yet. From 07e0187cf2de7ab526197a206d1e42efbfa4b779 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 22 Nov 2023 15:03:34 +0000 Subject: [PATCH 03/27] Add incremental schema updates --- nexus/db-model/src/schema.rs | 2 +- schema/crdb/14.0.0/up01.sql | 1 + schema/crdb/14.0.0/up02.sql | 3 +++ schema/crdb/14.0.0/up03.sql | 3 +++ schema/crdb/14.0.0/up04.sql | 4 ++++ schema/crdb/14.0.0/up05.sql | 1 + schema/crdb/14.0.0/up06.sql | 1 + 7 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 schema/crdb/14.0.0/up01.sql create mode 100644 schema/crdb/14.0.0/up02.sql create mode 100644 schema/crdb/14.0.0/up03.sql create mode 100644 schema/crdb/14.0.0/up04.sql create mode 100644 schema/crdb/14.0.0/up05.sql create mode 100644 schema/crdb/14.0.0/up06.sql diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index f5fc2eb567d..7ecaf9825a4 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -1290,7 +1290,7 @@ table! { /// /// This should be updated whenever the schema is changed. For more details, /// refer to: schema/crdb/README.adoc -pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(13, 0, 0); +pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(14, 0, 0); allow_tables_to_appear_in_same_query!( system_update, diff --git a/schema/crdb/14.0.0/up01.sql b/schema/crdb/14.0.0/up01.sql new file mode 100644 index 00000000000..6cfa92f4c24 --- /dev/null +++ b/schema/crdb/14.0.0/up01.sql @@ -0,0 +1 @@ +ALTER TABLE omicron.public.external_ip ADD COLUMN IF NOT EXISTS project_id UUID; diff --git a/schema/crdb/14.0.0/up02.sql b/schema/crdb/14.0.0/up02.sql new file mode 100644 index 00000000000..2c30d32b6bf --- /dev/null +++ b/schema/crdb/14.0.0/up02.sql @@ -0,0 +1,3 @@ +ALTER TABLE omicron.public.external_ip ADD CONSTRAINT null_project_id IF NOT EXISTS CHECK ( + kind = 'floating' OR project_id IS NULL +); diff --git a/schema/crdb/14.0.0/up03.sql b/schema/crdb/14.0.0/up03.sql new file mode 100644 index 00000000000..d41d45f9ec6 --- /dev/null +++ b/schema/crdb/14.0.0/up03.sql @@ -0,0 +1,3 @@ +ALTER TABLE omicron.public.external_ip ADD CONSTRAINT null_non_fip_pool_id IF NOT EXISTS CHECK ( + kind = 'floating' OR (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) +); diff --git a/schema/crdb/14.0.0/up04.sql b/schema/crdb/14.0.0/up04.sql new file mode 100644 index 00000000000..b912c5d9603 --- /dev/null +++ b/schema/crdb/14.0.0/up04.sql @@ -0,0 +1,4 @@ +ALTER TABLE omicron.public.external_ip ADD CONSTRAINT null_pool_range_id IF NOT EXISTS CHECK ( + (ip_pool_id IS NULL AND ip_pool_range_id IS NULL) OR + (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) +); diff --git a/schema/crdb/14.0.0/up05.sql b/schema/crdb/14.0.0/up05.sql new file mode 100644 index 00000000000..2447899934c --- /dev/null +++ b/schema/crdb/14.0.0/up05.sql @@ -0,0 +1 @@ +ALTER TABLE omicron.public.external_ip ALTER COLUMN ip_pool_id TYPE UUID; diff --git a/schema/crdb/14.0.0/up06.sql b/schema/crdb/14.0.0/up06.sql new file mode 100644 index 00000000000..844907d1bd3 --- /dev/null +++ b/schema/crdb/14.0.0/up06.sql @@ -0,0 +1 @@ +ALTER TABLE omicron.public.external_ip ALTER COLUMN ip_pool_range_id TYPE UUID; From b430952ffb0f1021d17b2496770d0c76cd8dd32a Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 22 Nov 2023 16:45:05 +0000 Subject: [PATCH 04/27] `cargo fmt`, some implementation post-it notes --- .../src/db/datastore/external_ip.rs | 47 +++++++++++++++++++ nexus/src/app/external_ip.rs | 21 +++++---- nexus/src/app/instance.rs | 2 - nexus/src/app/mod.rs | 5 +- nexus/src/app/sagas/instance_create.rs | 10 ++++ nexus/src/external_api/http_entrypoints.rs | 12 +++-- nexus/types/src/external_api/params.rs | 16 +++++-- 7 files changed, 92 insertions(+), 21 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index 766c49e991e..8756352a664 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -27,6 +27,8 @@ use nexus_types::identity::Resource; use omicron_common::api::external::CreateResult; use omicron_common::api::external::Error; use omicron_common::api::external::LookupResult; +use omicron_common::api::external::NameOrId; +use omicron_common::api::external::UpdateResult; use std::net::IpAddr; use uuid::Uuid; @@ -146,6 +148,51 @@ impl DataStore { self.allocate_external_ip(opctx, data).await } + /// Allocates a floating IP address for instance usage. + pub async fn attach_floating_ip_to_instance( + &self, + opctx: &OpContext, + instance_id: Uuid, + ip_id: &NameOrId, + project: &authz::Project, + ) -> UpdateResult { + use db::schema::external_ip::dsl; + // TODO: scope by project + // opctx.authorize(authz::Action::CreateChild, authz_project).await?; + let conn = self.pool_connection_authorized(opctx).await?; + + // let ip_id = match ip_id { + // NameOrId::Id(id) => *id, + // NameOrId::Name(name) => { + // diesel::select(dsl::external_ip) + // .filter(dsl::time_deleted.is_null()) + // .filter(dsl::name.eq(&name)) + // .execute_and_check(&conn) + // .await + // .map(|m| m.) + // } + // }; + + // opctx.authn. + // todo!() + + // diesel::update(dsl::external_ip) + // .filter(dsl::time_deleted.is_null()) + // .filter(dsl::id.eq(Some(ip_id))) + // .filter(dsl::parent_id.is_null()) + // .set(dsl::parent_id.eq(instance_id)) + // .check_if_exists::(ip_id) + // .execute_and_check(&*self.pool_connection_authorized(opctx).await?) + // .await + // .map(|r| match r.status { + // UpdateStatus::Updated => true, + // UpdateStatus::NotUpdatedButExists => false, + // }) + // .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + + todo!() + } + async fn allocate_external_ip( &self, opctx: &OpContext, diff --git a/nexus/src/app/external_ip.rs b/nexus/src/app/external_ip.rs index f3d1cb16bc5..9b1b8535a12 100644 --- a/nexus/src/app/external_ip.rs +++ b/nexus/src/app/external_ip.rs @@ -50,9 +50,7 @@ impl super::Nexus { .lookup_floating_ips(opctx, authz_project.id()) .await? .into_iter() - .map(|ip| { - ip.try_into().unwrap() - }) + .map(|ip| ip.try_into().unwrap()) .collect::>()) } @@ -66,9 +64,15 @@ impl super::Nexus { project_lookup.lookup_for(authz::Action::CreateChild).await?; let chosen_addr = match (¶ms.pool, params.address) { - (Some(_), _) => todo!("Drawing floating IP from pools not yet supported."), + (Some(_), _) => { + todo!("Drawing floating IP from pools not yet supported.") + } (None, Some(ip)) => ip, - _ => return Err(Error::invalid_request("floating IP needs a pool or ")), + _ => { + return Err(Error::invalid_request( + "floating IP needs a pool or ", + )) + } }; Ok(self @@ -79,9 +83,10 @@ impl super::Nexus { Uuid::new_v4(), ¶ms.identity.name.clone().into(), ¶ms.identity.description, - chosen_addr) + chosen_addr, + ) .await? - .try_into().unwrap() - ) + .try_into() + .unwrap()) } } diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 923bb1777ec..f704aa647ca 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -895,8 +895,6 @@ impl super::Nexus { .partition(|ip| ip.kind == IpKind::SNat); // Sanity checks on the number and kind of each IP address. - // TODO-correctness: Handle multiple IP addresses, see - // https://github.com/oxidecomputer/omicron/issues/1467 if external_ips.len() > MAX_EXTERNAL_IPS_PER_INSTANCE { return Err(Error::internal_error( format!( diff --git a/nexus/src/app/mod.rs b/nexus/src/app/mod.rs index 18c9dae8412..b04344b2a44 100644 --- a/nexus/src/app/mod.rs +++ b/nexus/src/app/mod.rs @@ -79,8 +79,9 @@ pub(crate) use nexus_db_queries::db::queries::disk::MAX_DISKS_PER_INSTANCE; pub(crate) const MAX_NICS_PER_INSTANCE: usize = 8; -// TODO-completeness: Support multiple external IPs -pub(crate) const MAX_EXTERNAL_IPS_PER_INSTANCE: usize = 1; +// XXX: Might want to recast as max *floating* IPs, we have at most one +// ephemeral (so bounded in saga by design). +pub(crate) const MAX_EXTERNAL_IPS_PER_INSTANCE: usize = 32; pub const MAX_VCPU_PER_INSTANCE: u16 = 64; diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs index 153e0323e77..87d9366780c 100644 --- a/nexus/src/app/sagas/instance_create.rs +++ b/nexus/src/app/sagas/instance_create.rs @@ -598,6 +598,8 @@ async fn sic_allocate_instance_snat_ip_undo( async fn sic_allocate_instance_external_ip( sagactx: NexusActionContext, ) -> Result<(), ActionError> { + // XXX: may wish to restructure partially: we have at most one ephemeral + // and then at most $n$ floating. let osagactx = sagactx.user_data(); let datastore = osagactx.datastore(); let repeat_saga_params = sagactx.saga_params::()?; @@ -622,6 +624,14 @@ async fn sic_allocate_instance_external_ip( params::ExternalIpCreate::Ephemeral { ref pool_name } => { pool_name.as_ref().map(|name| db::model::Name(name.clone())) } + params::ExternalIpCreate::Floating { ref floating_ip } => { + // In floating case, need: + // floating IP does not belong to another instance + // floating IP belongs to the parent project. + return Err(ActionError::action_failed(format!( + "can't yet bind floating ip {floating_ip:?} to instance" + ))); + } }; datastore .allocate_instance_ephemeral_ip(&opctx, ip_id, instance_id, pool_name) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 5e15ed964f2..0b7d8f2b88b 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1532,15 +1532,14 @@ async fn ip_pool_service_range_remove( async fn floating_ip_list( rqctx: RequestContext>, query_params: Query, -)-> Result>, HttpError> { +) -> Result>, HttpError> { let apictx = rqctx.context(); let handler = async { let nexus = &apictx.nexus; let opctx = crate::context::op_context_for_external_api(&rqctx).await?; let project_lookup = nexus.project_lookup(&opctx, query_params.into_inner())?; - let ips = - nexus.list_floating_ips(&opctx, &project_lookup).await?; + let ips = nexus.list_floating_ips(&opctx, &project_lookup).await?; Ok(HttpResponseOk(ResultsPage { items: ips, next_page: None })) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await @@ -1562,8 +1561,11 @@ async fn floating_ip_create( let floating_params = floating_params.into_inner(); let handler = async { let opctx = crate::context::op_context_for_external_api(&rqctx).await?; - let project_lookup = nexus.project_lookup(&opctx, query_params.into_inner())?; - let pool = nexus.create_floating_ip(&opctx, &project_lookup, &floating_params).await?; + let project_lookup = + nexus.project_lookup(&opctx, query_params.into_inner())?; + let pool = nexus + .create_floating_ip(&opctx, &project_lookup, &floating_params) + .await?; Ok(HttpResponseCreated(views::ExternalIp::from(pool))) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 1fa0e521798..3a3dabc9690 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -758,11 +758,15 @@ pub struct FloatingIpCreate { #[serde(flatten)] pub identity: IdentityMetadataCreateParams, - /// The intended address - // TODO: make non-optional and draw from pool + /// An IP address to reserve for use as a floating IP. This field is + /// optional if a pool is provided, in which case an address will + /// be automatically chosen from there. + // TODO: draw from pool if needed. pub address: Option, - /// The parent pool that a + /// The parent IP pool that a floating IP is pulled from. If combined + /// with an explicit address, then that address must be available in + /// the pool. // TODO: support tie-in to pools. pub pool: Option, } @@ -834,7 +838,11 @@ pub enum ExternalIpCreate { /// automatically-assigned from the provided IP Pool, or all available pools /// if not specified. Ephemeral { pool_name: Option }, - // TODO: Add floating IPs: https://github.com/oxidecomputer/omicron/issues/1334 + /// An IP address providing both inbound and outbound access. The address is + /// an existing Floating IP object assigned to the current project. + /// + /// The floating IP must not be in use by another instance or service. + Floating { floating_ip: NameOrId }, } /// Create-time parameters for an `Instance` From 4db90485de78fb2737d536efcaf1355230afe81e Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 22 Nov 2023 19:11:38 +0000 Subject: [PATCH 05/27] Fix schema migration and definitions --- nexus/db-model/src/external_ip.rs | 2 +- nexus/db-model/src/schema.rs | 2 +- nexus/db-queries/src/db/datastore/external_ip.rs | 10 +++++----- nexus/src/external_api/http_entrypoints.rs | 4 ++-- schema/crdb/14.0.0/up02.sql | 2 +- schema/crdb/14.0.0/up03.sql | 2 +- schema/crdb/14.0.0/up04.sql | 2 +- schema/crdb/dbinit.sql | 6 +++--- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/nexus/db-model/src/external_ip.rs b/nexus/db-model/src/external_ip.rs index 98960a6372f..5fdecc5337a 100644 --- a/nexus/db-model/src/external_ip.rs +++ b/nexus/db-model/src/external_ip.rs @@ -57,7 +57,6 @@ pub struct ExternalIp { pub time_created: DateTime, pub time_modified: DateTime, pub time_deleted: Option>, - pub project_id: Option, pub ip_pool_id: Option, pub ip_pool_range_id: Option, pub is_service: bool, @@ -70,6 +69,7 @@ pub struct ExternalIp { pub ip: IpNetwork, pub first_port: SqlU16, pub last_port: SqlU16, + pub project_id: Option, } impl From for sled_agent_client::types::SourceNatConfig { diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index 7ecaf9825a4..1820239fbde 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -524,7 +524,6 @@ table! { time_created -> Timestamptz, time_modified -> Timestamptz, time_deleted -> Nullable, - project_id -> Nullable, ip_pool_id -> Nullable, ip_pool_range_id -> Nullable, is_service -> Bool, @@ -533,6 +532,7 @@ table! { ip -> Inet, first_port -> Int4, last_port -> Int4, + project_id -> Nullable, } } diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index 8756352a664..fa7d936de60 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -152,14 +152,14 @@ impl DataStore { pub async fn attach_floating_ip_to_instance( &self, opctx: &OpContext, - instance_id: Uuid, - ip_id: &NameOrId, - project: &authz::Project, + _instance_id: Uuid, + _ip_id: &NameOrId, + _project: &authz::Project, ) -> UpdateResult { - use db::schema::external_ip::dsl; + // use db::schema::external_ip::dsl; // TODO: scope by project // opctx.authorize(authz::Action::CreateChild, authz_project).await?; - let conn = self.pool_connection_authorized(opctx).await?; + let _conn = self.pool_connection_authorized(opctx).await?; // let ip_id = match ip_id { // NameOrId::Id(id) => *id, diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 0b7d8f2b88b..843ffa4d60d 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1563,10 +1563,10 @@ async fn floating_ip_create( let opctx = crate::context::op_context_for_external_api(&rqctx).await?; let project_lookup = nexus.project_lookup(&opctx, query_params.into_inner())?; - let pool = nexus + let ip = nexus .create_floating_ip(&opctx, &project_lookup, &floating_params) .await?; - Ok(HttpResponseCreated(views::ExternalIp::from(pool))) + Ok(HttpResponseCreated(views::ExternalIp::from(ip))) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } diff --git a/schema/crdb/14.0.0/up02.sql b/schema/crdb/14.0.0/up02.sql index 2c30d32b6bf..9d07275113e 100644 --- a/schema/crdb/14.0.0/up02.sql +++ b/schema/crdb/14.0.0/up02.sql @@ -1,3 +1,3 @@ -ALTER TABLE omicron.public.external_ip ADD CONSTRAINT null_project_id IF NOT EXISTS CHECK ( +ALTER TABLE omicron.public.external_ip ADD CONSTRAINT IF NOT EXISTS null_project_id CHECK ( kind = 'floating' OR project_id IS NULL ); diff --git a/schema/crdb/14.0.0/up03.sql b/schema/crdb/14.0.0/up03.sql index d41d45f9ec6..692770425c8 100644 --- a/schema/crdb/14.0.0/up03.sql +++ b/schema/crdb/14.0.0/up03.sql @@ -1,3 +1,3 @@ -ALTER TABLE omicron.public.external_ip ADD CONSTRAINT null_non_fip_pool_id IF NOT EXISTS CHECK ( +ALTER TABLE omicron.public.external_ip ADD CONSTRAINT IF NOT EXISTS null_non_fip_pool_id CHECK ( kind = 'floating' OR (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) ); diff --git a/schema/crdb/14.0.0/up04.sql b/schema/crdb/14.0.0/up04.sql index b912c5d9603..742b83a9e6a 100644 --- a/schema/crdb/14.0.0/up04.sql +++ b/schema/crdb/14.0.0/up04.sql @@ -1,4 +1,4 @@ -ALTER TABLE omicron.public.external_ip ADD CONSTRAINT null_pool_range_id IF NOT EXISTS CHECK ( +ALTER TABLE omicron.public.external_ip ADD CONSTRAINT IF NOT EXISTS null_pool_range_id CHECK ( (ip_pool_id IS NULL AND ip_pool_range_id IS NULL) OR (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) ); diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 7d451dfd013..3eb07aeede8 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -1578,9 +1578,6 @@ CREATE TABLE IF NOT EXISTS omicron.public.external_ip ( time_modified TIMESTAMPTZ NOT NULL, time_deleted TIMESTAMPTZ, - /* FK to the `ip_pool` table. */ - project_id UUID, - /* FK to the `ip_pool` table. */ ip_pool_id UUID, @@ -1605,6 +1602,9 @@ CREATE TABLE IF NOT EXISTS omicron.public.external_ip ( /* The last port in the allowed range, also inclusive. */ last_port INT4 NOT NULL, + /* FK to the `ip_pool` table. */ + project_id UUID, + /* The name must be non-NULL iff this is a floating IP. */ CONSTRAINT null_fip_name CHECK ( (kind != 'floating' AND name IS NULL) OR From ba0f7a3de272de1ac4277f93a8ea45135b582c14 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Thu, 23 Nov 2023 14:43:36 +0000 Subject: [PATCH 06/27] Working addition/listing of floating IPs, correct DB constraints --- common/src/api/external/mod.rs | 1 + nexus/db-model/src/external_ip.rs | 26 +++--- nexus/db-model/src/schema.rs | 6 +- .../src/db/datastore/external_ip.rs | 48 ++++++++-- nexus/db-queries/src/db/datastore/ip_pool.rs | 10 +++ nexus/db-queries/src/db/datastore/mod.rs | 12 +-- nexus/db-queries/src/db/datastore/rack.rs | 16 ++-- nexus/db-queries/src/db/lookup.rs | 10 +++ .../db-queries/src/db/queries/external_ip.rs | 12 ++- nexus/src/app/external_ip.rs | 72 ++++++++++++--- nexus/src/external_api/http_entrypoints.rs | 88 +++++++++++++++++-- nexus/src/external_api/tag-config.json | 6 ++ nexus/types/src/external_api/params.rs | 9 ++ schema/crdb/14.0.0/up02.sql | 3 +- schema/crdb/14.0.0/up03.sql | 9 +- schema/crdb/14.0.0/up04.sql | 11 ++- schema/crdb/14.0.0/up05.sql | 1 - schema/crdb/14.0.0/up06.sql | 1 - schema/crdb/dbinit.sql | 36 +++++--- 19 files changed, 298 insertions(+), 79 deletions(-) delete mode 100644 schema/crdb/14.0.0/up05.sql delete mode 100644 schema/crdb/14.0.0/up06.sql diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index adf661516a9..b83fb0ce2ca 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -751,6 +751,7 @@ pub enum ResourceType { Zpool, Vmm, Ipv4NatEntry, + // ExternalIp, } // IDENTITY METADATA diff --git a/nexus/db-model/src/external_ip.rs b/nexus/db-model/src/external_ip.rs index 5fdecc5337a..583bc5c2a2b 100644 --- a/nexus/db-model/src/external_ip.rs +++ b/nexus/db-model/src/external_ip.rs @@ -57,8 +57,8 @@ pub struct ExternalIp { pub time_created: DateTime, pub time_modified: DateTime, pub time_deleted: Option>, - pub ip_pool_id: Option, - pub ip_pool_range_id: Option, + pub ip_pool_id: Uuid, + pub ip_pool_range_id: Uuid, pub is_service: bool, // This is Some(_) for: // - all instance/service SNAT IPs @@ -69,6 +69,7 @@ pub struct ExternalIp { pub ip: IpNetwork, pub first_port: SqlU16, pub last_port: SqlU16, + // Only Some(_) for instance Floating IPs pub project_id: Option, } @@ -93,7 +94,7 @@ pub struct IncompleteExternalIp { kind: IpKind, is_service: bool, parent_id: Option, - pool_id: Option, + pool_id: Uuid, project_id: Option, // Optional address requesting that a specific IP address be allocated. explicit_ip: Option, @@ -115,7 +116,7 @@ impl IncompleteExternalIp { kind: IpKind::SNat, is_service: false, parent_id: Some(instance_id), - pool_id: Some(pool_id), + pool_id, project_id: None, explicit_ip: None, explicit_port_range: None, @@ -131,7 +132,7 @@ impl IncompleteExternalIp { kind: IpKind::Ephemeral, is_service: false, parent_id: Some(instance_id), - pool_id: Some(pool_id), + pool_id, project_id: None, explicit_ip: None, explicit_port_range: None, @@ -153,7 +154,7 @@ impl IncompleteExternalIp { kind: IpKind::Floating, is_service: false, parent_id: None, - pool_id: Some(pool_id), + pool_id, project_id: Some(project_id), explicit_ip: None, explicit_port_range: None, @@ -166,6 +167,7 @@ impl IncompleteExternalIp { description: &str, project_id: Uuid, explicit_ip: IpAddr, + pool_id: Uuid, ) -> Self { Self { id, @@ -175,7 +177,7 @@ impl IncompleteExternalIp { kind: IpKind::Floating, is_service: false, parent_id: None, - pool_id: None, + pool_id, project_id: Some(project_id), explicit_ip: Some(explicit_ip.into()), explicit_port_range: None, @@ -198,7 +200,7 @@ impl IncompleteExternalIp { kind: IpKind::Floating, is_service: true, parent_id: Some(service_id), - pool_id: Some(pool_id), + pool_id, project_id: None, explicit_ip: Some(IpNetwork::from(address)), explicit_port_range: None, @@ -227,7 +229,7 @@ impl IncompleteExternalIp { kind: IpKind::SNat, is_service: true, parent_id: Some(service_id), - pool_id: Some(pool_id), + pool_id, project_id: None, explicit_ip: Some(IpNetwork::from(address)), explicit_port_range, @@ -249,7 +251,7 @@ impl IncompleteExternalIp { kind: IpKind::Floating, is_service: true, parent_id: Some(service_id), - pool_id: Some(pool_id), + pool_id, project_id: None, explicit_ip: None, explicit_port_range: None, @@ -265,7 +267,7 @@ impl IncompleteExternalIp { kind: IpKind::SNat, is_service: true, parent_id: Some(service_id), - pool_id: Some(pool_id), + pool_id, project_id: None, explicit_ip: None, explicit_port_range: None, @@ -300,7 +302,7 @@ impl IncompleteExternalIp { &self.parent_id } - pub fn pool_id(&self) -> &Option { + pub fn pool_id(&self) -> &Uuid { &self.pool_id } diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index 1820239fbde..1449c3d20eb 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -524,14 +524,16 @@ table! { time_created -> Timestamptz, time_modified -> Timestamptz, time_deleted -> Nullable, - ip_pool_id -> Nullable, - ip_pool_range_id -> Nullable, + + ip_pool_id -> Uuid, + ip_pool_range_id -> Uuid, is_service -> Bool, parent_id -> Nullable, kind -> crate::IpKindEnum, ip -> Inet, first_port -> Int4, last_port -> Int4, + project_id -> Nullable, } } diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index fa7d936de60..4fe0c6033ad 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -132,20 +132,52 @@ impl DataStore { pub async fn allocate_floating_ip( &self, opctx: &OpContext, + pool_id: Option, project_id: Uuid, ip_id: Uuid, name: &Name, description: &str, - ip: IpAddr, + ip: Option, ) -> CreateResult { - let data = IncompleteExternalIp::for_floating_explicit( - ip_id, - name, - description, - project_id, - ip, - ); + // XXX: mux here to scan *all* project pools in + // current silo for convenience? + let pool_id = if let Some(id) = pool_id { + id + } else { + self.ip_pools_fetch_default(opctx).await?.id() + }; + + // XXX: Verify that chosen pool comes from my silo. + + let data = if let Some(ip) = ip { + IncompleteExternalIp::for_floating_explicit( + ip_id, + name, + description, + project_id, + ip, + pool_id, + ) + } else { + IncompleteExternalIp::for_floating( + ip_id, + name, + description, + project_id, + pool_id, + ) + }; + + // TODO: need to disambiguate no IP and/or IP taken + // from resource name collision, and expose those in + // a nice way. self.allocate_external_ip(opctx, data).await + // .map_err(|e| { + // public_error_from_diesel( + // e, + // ErrorHandler::Conflict(todo!(), name.as_str()) + // ) + // }) } /// Allocates a floating IP address for instance usage. diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index fb300ef8338..6be134fd652 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -110,6 +110,16 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + /// Lookup an IP pool within the current silo which contains a target IP + /// address. + pub async fn ip_pools_fetch_for_ip( + &self, + opctx: &OpContext, + ip_addr: std::net::IpAddr, + ) -> LookupResult { + todo!() + } + /// Looks up an IP pool intended for internal services. /// /// This method may require an index by Availability Zone in the future. diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index 22b23cbef31..2456c798431 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -1565,8 +1565,8 @@ mod test { time_created: now, time_modified: now, time_deleted: None, - ip_pool_id: Some(Uuid::new_v4()), - ip_pool_range_id: Some(Uuid::new_v4()), + ip_pool_id: Uuid::new_v4(), + ip_pool_range_id: Uuid::new_v4(), project_id: None, is_service: false, parent_id: Some(instance_id), @@ -1626,8 +1626,8 @@ mod test { time_created: now, time_modified: now, time_deleted: None, - ip_pool_id: Some(Uuid::new_v4()), - ip_pool_range_id: Some(Uuid::new_v4()), + ip_pool_id: Uuid::new_v4(), + ip_pool_range_id: Uuid::new_v4(), project_id: None, is_service: false, parent_id: Some(Uuid::new_v4()), @@ -1697,8 +1697,8 @@ mod test { time_created: now, time_modified: now, time_deleted: None, - ip_pool_id: Some(Uuid::new_v4()), - ip_pool_range_id: Some(Uuid::new_v4()), + ip_pool_id: Uuid::new_v4(), + ip_pool_range_id: Uuid::new_v4(), project_id: None, is_service: false, parent_id: Some(Uuid::new_v4()), diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index 1c8f2f1f04e..2cc58804702 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -1148,31 +1148,31 @@ mod test { assert_eq!(observed_ip_pool_ranges[0].ip_pool_id, svc_pool.id()); // Verify the allocated external IPs - assert_eq!(dns_external_ip.ip_pool_id, Some(svc_pool.id())); + assert_eq!(dns_external_ip.ip_pool_id, svc_pool.id()); assert_eq!( dns_external_ip.ip_pool_range_id, - Some(observed_ip_pool_ranges[0].id) + observed_ip_pool_ranges[0].id ); assert_eq!(dns_external_ip.ip.ip(), external_dns_ip); - assert_eq!(nexus_external_ip.ip_pool_id, Some(svc_pool.id())); + assert_eq!(nexus_external_ip.ip_pool_id, svc_pool.id()); assert_eq!( nexus_external_ip.ip_pool_range_id, - Some(observed_ip_pool_ranges[0].id) + observed_ip_pool_ranges[0].id ); assert_eq!(nexus_external_ip.ip.ip(), nexus_ip); - assert_eq!(ntp1_external_ip.ip_pool_id, Some(svc_pool.id())); + assert_eq!(ntp1_external_ip.ip_pool_id, svc_pool.id()); assert_eq!( ntp1_external_ip.ip_pool_range_id, - Some(observed_ip_pool_ranges[0].id) + observed_ip_pool_ranges[0].id ); assert_eq!(ntp1_external_ip.ip.ip(), ntp1_ip); - assert_eq!(ntp2_external_ip.ip_pool_id, Some(svc_pool.id())); + assert_eq!(ntp2_external_ip.ip_pool_id, svc_pool.id()); assert_eq!( ntp2_external_ip.ip_pool_range_id, - Some(observed_ip_pool_ranges[0].id) + observed_ip_pool_ranges[0].id ); assert_eq!(ntp2_external_ip.ip.ip(), ntp2_ip); diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index 72a32f562c9..ead2350a506 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -632,6 +632,7 @@ lookup_resource! { lookup_resource! { name = "Project", ancestors = [ "Silo" ], + // children = [ "Disk", "Instance", "Vpc", "Snapshot", "ProjectImage", "ExternalIp" ], children = [ "Disk", "Instance", "Vpc", "Snapshot", "ProjectImage" ], lookup_by_name = true, soft_deletes = true, @@ -728,6 +729,15 @@ lookup_resource! { primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } +// lookup_resource! { +// name = "ExternalIp", +// ancestors = [ "Silo", "Project" ], +// children = [], +// lookup_by_name = true, +// soft_deletes = true, +// primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] +// } + // Miscellaneous resources nested directly below "Fleet" lookup_resource! { diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index f93b26b9efb..4e5f59e79c6 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -98,7 +98,8 @@ const MAX_PORT: u16 = u16::MAX; /// AS kind, /// candidate_ip AS ip, /// CAST(candidate_first_port AS INT4) AS first_port, -/// CAST(candidate_last_port AS INT4) AS last_port +/// CAST(candidate_last_port AS INT4) AS last_port, +/// AS project_id /// FROM /// SELECT * FROM ( /// -- Select all IP addresses by pool and range. @@ -371,6 +372,13 @@ impl NextExternalIp { out.push_identifier(dsl::first_port::NAME)?; out.push_sql(", CAST(candidate_last_port AS INT4) AS "); out.push_identifier(dsl::last_port::NAME)?; + out.push_sql(", "); + + // Project ID, possibly null + out.push_bind_param::, Option>(self.ip.project_id())?; + out.push_sql(" AS "); + out.push_identifier(dsl::project_id::NAME)?; + out.push_sql(" FROM ("); self.push_address_sequence_subquery(out.reborrow())?; out.push_sql(") CROSS JOIN ("); @@ -622,7 +630,7 @@ impl NextExternalIp { out.push_sql(" WHERE "); out.push_identifier(dsl::ip_pool_id::NAME)?; out.push_sql(" = "); - out.push_bind_param::, Option>(self.ip.pool_id())?; + out.push_bind_param::(self.ip.pool_id())?; out.push_sql(" AND "); out.push_identifier(dsl::time_deleted::NAME)?; out.push_sql(" IS NULL"); diff --git a/nexus/src/app/external_ip.rs b/nexus/src/app/external_ip.rs index 9b1b8535a12..0c3a577fda1 100644 --- a/nexus/src/app/external_ip.rs +++ b/nexus/src/app/external_ip.rs @@ -8,11 +8,15 @@ use crate::external_api::views::ExternalIp; use nexus_db_queries::authz; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::lookup; +use nexus_db_queries::db::lookup::LookupPath; use nexus_db_queries::db::model::IpKind; use nexus_types::external_api::params; use omicron_common::api::external::CreateResult; +use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; +use omicron_common::api::external::LookupResult; +use omicron_common::api::external::NameOrId; use uuid::Uuid; impl super::Nexus { @@ -38,10 +42,45 @@ impl super::Nexus { .collect::>()) } - pub(crate) async fn list_floating_ips( + pub fn floating_ip_lookup<'a>( + &'a self, + opctx: &'a OpContext, + fip_selector: params::FloatingIpSelector, + ) -> LookupResult> { + match fip_selector { + params::FloatingIpSelector { floating_ip: NameOrId::Id(id), project: None } => { + // let floating_ip = + // LookupPath::new(opctx, &self.db_datastore).floating_ip_id(id); + // Ok(floating_ip) + todo!() + } + params::FloatingIpSelector { + floating_ip: NameOrId::Name(name), + project: Some(project), + } => { + // let floating_ip = self + // .project_lookup(opctx, params::ProjectSelector { project })? + // .floating_ip_name_owned(name.into()); + // Ok(floating_ip) + todo!() + } + params::FloatingIpSelector { + floating_ip: NameOrId::Id(_), + .. + } => Err(Error::invalid_request( + "when providing Floating IP as an ID project should not be specified", + )), + _ => Err(Error::invalid_request( + "Floating IP should either be UUID or project should be specified", + )), + } + } + + pub(crate) async fn floating_ips_list( &self, opctx: &OpContext, project_lookup: &lookup::Project<'_>, + // pagparams: &PaginatedBy<'_>, ) -> ListResultVec { let (.., authz_project) = project_lookup.lookup_for(authz::Action::Read).await?; @@ -54,7 +93,7 @@ impl super::Nexus { .collect::>()) } - pub(crate) async fn create_floating_ip( + pub(crate) async fn floating_ip_create( &self, opctx: &OpContext, project_lookup: &lookup::Project<'_>, @@ -63,30 +102,41 @@ impl super::Nexus { let (.., authz_project) = project_lookup.lookup_for(authz::Action::CreateChild).await?; - let chosen_addr = match (¶ms.pool, params.address) { - (Some(_), _) => { - todo!("Drawing floating IP from pools not yet supported.") - } - (None, Some(ip)) => ip, - _ => { - return Err(Error::invalid_request( - "floating IP needs a pool or ", + // XXX: support pool by name here. + let pool_id = match ¶ms.pool { + Some(NameOrId::Id(ref id)) => Some(*id), + Some(NameOrId::Name(ref _name)) => { + return Err(Error::internal_error( + "pool ref by name not yet supported", )) } + None => None, }; Ok(self .db_datastore .allocate_floating_ip( opctx, + pool_id, authz_project.id(), Uuid::new_v4(), ¶ms.identity.name.clone().into(), ¶ms.identity.description, - chosen_addr, + params.address, ) .await? .try_into() .unwrap()) } + + pub(crate) async fn floating_ip_delete( + &self, + opctx: &OpContext, + // pool_lookup: &lookup::<'_>, + ) -> DeleteResult { + // let (.., authz_pool, db_pool) = + // pool_lookup.fetch_for(authz::Action::Delete).await?; + // self.db_datastore.ip_pool_delete(opctx, &authz_pool, &db_pool).await + todo!() + } } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 843ffa4d60d..aca6278f12d 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -141,6 +141,8 @@ pub(crate) fn external_api() -> NexusApiDescription { api.register(floating_ip_list)?; api.register(floating_ip_create)?; + api.register(floating_ip_view)?; + api.register(floating_ip_delete)?; api.register(disk_list)?; api.register(disk_create)?; @@ -1526,21 +1528,32 @@ async fn ip_pool_service_range_remove( /// List all Floating IPs #[endpoint { method = GET, - path = "/v1/system/floating-ips", - tags = ["system/networking"], + path = "/v1/floating-ips", + tags = ["floating-ips"], }] async fn floating_ip_list( rqctx: RequestContext>, - query_params: Query, + query_params: Query>, ) -> Result>, HttpError> { let apictx = rqctx.context(); let handler = async { let nexus = &apictx.nexus; let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + let query = query_params.into_inner(); + let pag_params = data_page_params_for(&rqctx, &query)?; + let scan_params = ScanByNameOrId::from_query(&query)?; + let paginated_by = name_or_id_pagination(&pag_params, scan_params)?; let project_lookup = - nexus.project_lookup(&opctx, query_params.into_inner())?; - let ips = nexus.list_floating_ips(&opctx, &project_lookup).await?; + nexus.project_lookup(&opctx, scan_params.selector.clone())?; + // XXX: impl relevant traits to make results_page work. + // let ips = nexus.list_floating_ips(&opctx, &project_lookup, &paginated_by).await?; + let ips = nexus.floating_ips_list(&opctx, &project_lookup).await?; Ok(HttpResponseOk(ResultsPage { items: ips, next_page: None })) + // Ok(HttpResponseOk(ScanByNameOrId::results_page( + // &query, + // ips, + // &marker_for_name_or_id, + // )?)) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } @@ -1548,8 +1561,8 @@ async fn floating_ip_list( /// Create a Floating IP #[endpoint { method = POST, - path = "/v1/system/floating-ips", - tags = ["system/networking"], + path = "/v1/floating-ips", + tags = ["floating-ips"], }] async fn floating_ip_create( rqctx: RequestContext>, @@ -1564,13 +1577,72 @@ async fn floating_ip_create( let project_lookup = nexus.project_lookup(&opctx, query_params.into_inner())?; let ip = nexus - .create_floating_ip(&opctx, &project_lookup, &floating_params) + .floating_ip_create(&opctx, &project_lookup, &floating_params) .await?; Ok(HttpResponseCreated(views::ExternalIp::from(ip))) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +/// Delete a Floating IP +#[endpoint { + method = DELETE, + path = "/v1/floating-ips/{floating_ip}", + tags = ["floating-ips"], +}] +async fn floating_ip_delete( + rqctx: RequestContext>, + path_params: Path, + query_params: Query, +) -> Result { + todo!(); + + // TODO: more work needed here to plug into authz, and pathlookup + // etc. + + // let apictx = rqctx.context(); + // let handler = async { + // let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + // let nexus = &apictx.nexus; + // let path = path_params.into_inner(); + // let query = query_params.into_inner(); + // let disk_selector = + // params::FloatingIpSelector { floating_ip: path.floating_ip, project: query.project }; + // let disk_lookup = nexus.disk_lookup(&opctx, disk_selector)?; + // nexus.project_delete_disk(&opctx, &disk_lookup).await?; + // Ok(HttpResponseDeleted()) + // }; + // apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/// Fetch a floating IP +#[endpoint { + method = GET, + path = "/v1/floating-ips/{floating_ip}", + tags = ["floating-ips"] +}] +async fn floating_ip_view( + rqctx: RequestContext>, + path_params: Path, + query_params: Query, +) -> Result, HttpError> { + todo!(); + + // let apictx = rqctx.context(); + // let handler = async { + // let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + // let nexus = &apictx.nexus; + // let path = path_params.into_inner(); + // let query = query_params.into_inner(); + // let disk_selector = + // params::FloatingIpSelector { floating_ip: path.floating_ip, project: query.project }; + // let (.., disk) = + // nexus.disk_lookup(&opctx, disk_selector)?.fetch().await?; + // Ok(HttpResponseOk(disk.into())) + // }; + // apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + // Disks /// List disks diff --git a/nexus/src/external_api/tag-config.json b/nexus/src/external_api/tag-config.json index 07eb198016f..3bc8006cee8 100644 --- a/nexus/src/external_api/tag-config.json +++ b/nexus/src/external_api/tag-config.json @@ -8,6 +8,12 @@ "url": "http://docs.oxide.computer/api/disks" } }, + "floating-ips": { + "description": "Floating IPs allow a project to allocate well-known IPs to instances.", + "external_docs": { + "url": "http://docs.oxide.computer/api/floating-ips" + } + }, "hidden": { "description": "TODO operations that will not ship to customers", "external_docs": { diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 3a3dabc9690..0c24570e570 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -54,6 +54,7 @@ path_param!(VpcPath, vpc, "VPC"); path_param!(SubnetPath, subnet, "subnet"); path_param!(RouterPath, router, "router"); path_param!(RoutePath, route, "route"); +path_param!(FloatingIpPath, floating_ip, "Floating IP"); path_param!(DiskPath, disk, "disk"); path_param!(SnapshotPath, snapshot, "snapshot"); path_param!(ImagePath, image, "image"); @@ -129,6 +130,14 @@ pub struct OptionalProjectSelector { pub project: Option, } +#[derive(Deserialize, JsonSchema)] +pub struct FloatingIpSelector { + /// Name or ID of the project, only required if `floating_ip` is provided as a `Name` + pub project: Option, + /// Name or ID of the Floating IP + pub floating_ip: NameOrId, +} + #[derive(Deserialize, JsonSchema)] pub struct DiskSelector { /// Name or ID of the project, only required if `disk` is provided as a `Name` diff --git a/schema/crdb/14.0.0/up02.sql b/schema/crdb/14.0.0/up02.sql index 9d07275113e..733c46b0dc2 100644 --- a/schema/crdb/14.0.0/up02.sql +++ b/schema/crdb/14.0.0/up02.sql @@ -1,3 +1,4 @@ ALTER TABLE omicron.public.external_ip ADD CONSTRAINT IF NOT EXISTS null_project_id CHECK ( - kind = 'floating' OR project_id IS NULL + (kind = 'floating' AND is_service = FALSE AND project_id IS NOT NULL) OR + ((kind != 'floating' OR is_service = TRUE) AND project_id IS NULL) ); diff --git a/schema/crdb/14.0.0/up03.sql b/schema/crdb/14.0.0/up03.sql index 692770425c8..d3577edc12a 100644 --- a/schema/crdb/14.0.0/up03.sql +++ b/schema/crdb/14.0.0/up03.sql @@ -1,3 +1,6 @@ -ALTER TABLE omicron.public.external_ip ADD CONSTRAINT IF NOT EXISTS null_non_fip_pool_id CHECK ( - kind = 'floating' OR (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) -); +CREATE UNIQUE INDEX IF NOT EXISTS lookup_floating_ip_by_name on omicron.public.external_ip ( + name +) WHERE + kind = 'floating' AND + time_deleted is NULL AND + project_id is NULL; diff --git a/schema/crdb/14.0.0/up04.sql b/schema/crdb/14.0.0/up04.sql index 742b83a9e6a..9a40dc99c57 100644 --- a/schema/crdb/14.0.0/up04.sql +++ b/schema/crdb/14.0.0/up04.sql @@ -1,4 +1,7 @@ -ALTER TABLE omicron.public.external_ip ADD CONSTRAINT IF NOT EXISTS null_pool_range_id CHECK ( - (ip_pool_id IS NULL AND ip_pool_range_id IS NULL) OR - (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) -); +CREATE UNIQUE INDEX IF NOT EXISTS lookup_floating_ip_by_name_and_project on omicron.public.external_ip ( + project_id, + name +) WHERE + kind = 'floating' AND + time_deleted is NULL AND + project_id is NOT NULL; diff --git a/schema/crdb/14.0.0/up05.sql b/schema/crdb/14.0.0/up05.sql deleted file mode 100644 index 2447899934c..00000000000 --- a/schema/crdb/14.0.0/up05.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE omicron.public.external_ip ALTER COLUMN ip_pool_id TYPE UUID; diff --git a/schema/crdb/14.0.0/up06.sql b/schema/crdb/14.0.0/up06.sql deleted file mode 100644 index 844907d1bd3..00000000000 --- a/schema/crdb/14.0.0/up06.sql +++ /dev/null @@ -1 +0,0 @@ -ALTER TABLE omicron.public.external_ip ALTER COLUMN ip_pool_range_id TYPE UUID; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 3eb07aeede8..1c31bcc31b9 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -1579,10 +1579,10 @@ CREATE TABLE IF NOT EXISTS omicron.public.external_ip ( time_deleted TIMESTAMPTZ, /* FK to the `ip_pool` table. */ - ip_pool_id UUID, + ip_pool_id UUID NOT NULL, /* FK to the `ip_pool_range` table. */ - ip_pool_range_id UUID, + ip_pool_range_id UUID NOT NULL, /* True if this IP is associated with a service rather than an instance. */ is_service BOOL NOT NULL, @@ -1602,7 +1602,7 @@ CREATE TABLE IF NOT EXISTS omicron.public.external_ip ( /* The last port in the allowed range, also inclusive. */ last_port INT4 NOT NULL, - /* FK to the `ip_pool` table. */ + /* FK to the `project` table. */ project_id UUID, /* The name must be non-NULL iff this is a floating IP. */ @@ -1617,11 +1617,12 @@ CREATE TABLE IF NOT EXISTS omicron.public.external_ip ( (kind = 'floating' AND description IS NOT NULL) ), - /* Only floating IPs can be attached to a project. - * Projects are nullable in such a case. + /* Only floating IPs can be attached to a project, and + * they must have a parent project if they are instance FIPs. */ CONSTRAINT null_project_id CHECK ( - kind = 'floating' OR project_id IS NULL + (kind = 'floating' AND is_service = FALSE AND project_id is NOT NULL) OR + ((kind != 'floating' OR is_service = TRUE) AND project_id IS NULL) ), /* Ephemeral/SNAT IPs must have a parent pool and range, while this @@ -1631,12 +1632,6 @@ CREATE TABLE IF NOT EXISTS omicron.public.external_ip ( kind = 'floating' OR (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) ), - /* If the IP pool is defined, the IP pool range must also be. */ - CONSTRAINT null_pool_range_id CHECK ( - (ip_pool_id IS NULL AND ip_pool_range_id IS NULL) OR - (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) - ), - /* * Only nullable if this is a floating IP, which may exist not * attached to any instance or service yet. @@ -1680,6 +1675,23 @@ CREATE UNIQUE INDEX IF NOT EXISTS lookup_external_ip_by_parent ON omicron.public ) WHERE parent_id IS NOT NULL AND time_deleted IS NULL; +/* Enforce name-uniqueness of floating (service) IPs at fleet level. */ +CREATE UNIQUE INDEX IF NOT EXISTS lookup_floating_ip_by_name on omicron.public.external_ip ( + name +) WHERE + kind = 'floating' AND + time_deleted is NULL AND + project_id is NULL; + +/* Enforce name-uniqueness of floating IPs at project level. */ +CREATE UNIQUE INDEX IF NOT EXISTS lookup_floating_ip_by_name_and_project on omicron.public.external_ip ( + project_id, + name +) WHERE + kind = 'floating' AND + time_deleted is NULL AND + project_id is NOT NULL; + /*******************************************************************/ /* From afb7f932841cd2d2ac8ed2a3f45e01bac36212ce Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Thu, 23 Nov 2023 17:19:39 +0000 Subject: [PATCH 07/27] Sled-agent + sled-agent-client plumbing of extIPs Remaining parts for baseline functionality are in the instance-create sagas. --- illumos-utils/src/opte/port_manager.rs | 38 +-- nexus/src/app/instance.rs | 28 ++- nexus/src/external_api/http_entrypoints.rs | 2 +- nexus/tests/output/nexus_tags.txt | 7 + openapi/nexus.json | 255 +++++++++++++++++++++ openapi/sled-agent.json | 17 +- sled-agent/src/instance.rs | 19 +- sled-agent/src/params.rs | 3 +- sled-agent/src/services.rs | 16 +- 9 files changed, 347 insertions(+), 38 deletions(-) diff --git a/illumos-utils/src/opte/port_manager.rs b/illumos-utils/src/opte/port_manager.rs index bf549bd4ffb..3558ef1c781 100644 --- a/illumos-utils/src/opte/port_manager.rs +++ b/illumos-utils/src/opte/port_manager.rs @@ -100,7 +100,8 @@ impl PortManager { &self, nic: &NetworkInterface, source_nat: Option, - external_ips: &[IpAddr], + ephemeral_ip: Option, + floating_ips: &[IpAddr], firewall_rules: &[VpcFirewallRule], dhcp_config: DhcpCfg, ) -> Result<(Port, PortTicket), Error> { @@ -112,15 +113,6 @@ impl PortManager { let boundary_services = default_boundary_services(); // Describe the external IP addresses for this port. - // - // Note that we're currently only taking the first address, which is all - // that OPTE supports. The array is guaranteed to be limited by Nexus. - // See https://github.com/oxidecomputer/omicron/issues/1467 - // See https://github.com/oxidecomputer/opte/issues/196 - - // XXX: Need to take ephemeral vs floating as separate params. - let external_ip = external_ips.get(0); - macro_rules! ip_cfg { ($ip:expr, $log_prefix:literal, $ip_t:path, $cidr_t:path, $ipcfg_e:path, $ipcfg_t:ident, $snat_t:ident) => {{ @@ -155,18 +147,33 @@ impl PortManager { } None => None, }; - let ephemeral_ip = match external_ip { - Some($ip_t(ip)) => Some((*ip).into()), + let ephemeral_ip = match ephemeral_ip { + Some($ip_t(ip)) => Some(ip.into()), Some(_) => { error!( self.inner.log, - concat!($log_prefix, " external IP"); - "external_ip" => ?external_ip, + concat!($log_prefix, " ephemeral IP"); + "ephemeral_ip" => ?ephemeral_ip, ); return Err(Error::InvalidPortIpConfig); } None => None, }; + let floating_ips: Vec<_> = floating_ips + .iter() + .copied() + .map(|ip| match ip { + $ip_t(ip) => Ok(ip.into()), + _ => { + error!( + self.inner.log, + concat!($log_prefix, " ephemeral IP"); + "ephemeral_ip" => ?ephemeral_ip, + ); + Err(Error::InvalidPortIpConfig) + } + }) + .collect::, _>>()?; $ipcfg_e($ipcfg_t { vpc_subnet, @@ -175,8 +182,7 @@ impl PortManager { external_ips: ExternalIpCfg { ephemeral_ip, snat, - // TODO: implement - floating_ips: vec![], + floating_ips, }, }) }} diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index f704aa647ca..21461b87800 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -885,8 +885,6 @@ impl super::Nexus { .await?; // Collect the external IPs for the instance. - // TODO-correctness: Handle Floating IPs, see - // https://github.com/oxidecomputer/omicron/issues/1334 let (snat_ip, external_ips): (Vec<_>, Vec<_>) = self .db_datastore .instance_lookup_external_ips(&opctx, authz_instance.id()) @@ -906,8 +904,27 @@ impl super::Nexus { .as_str(), )); } - let external_ips = - external_ips.into_iter().map(|model| model.ip.ip()).collect(); + + // Partition remaining external IPs by class: we can have at most + // one ephemeral ip. + let (ephemeral_ips, floating_ips): (Vec<_>, Vec<_>) = external_ips + .into_iter() + .partition(|ip| ip.kind == IpKind::Ephemeral); + + if ephemeral_ips.len() > 1 { + return Err(Error::internal_error( + format!( + "Expected at most one ephemeral IP for an instance, found {}", + ephemeral_ips.len() + ) + .as_str(), + )); + } + + let ephemeral_ip = ephemeral_ips.get(0).map(|model| model.ip.ip()); + + let floating_ips = + floating_ips.into_iter().map(|model| model.ip.ip()).collect(); if snat_ip.len() != 1 { return Err(Error::internal_error( "Expected exactly one SNAT IP address for an instance", @@ -983,7 +1000,8 @@ impl super::Nexus { }, nics, source_nat, - external_ips, + ephemeral_ip, + floating_ips, firewall_rules, dhcp_config: sled_agent_client::types::DhcpConfig { dns_servers: self.external_dns_servers.clone(), diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index aca6278f12d..dfa686d4339 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1625,7 +1625,7 @@ async fn floating_ip_view( rqctx: RequestContext>, path_params: Path, query_params: Query, -) -> Result, HttpError> { +) -> Result, HttpError> { todo!(); // let apictx = rqctx.context(); diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index 7f0c30c471f..eb17f285290 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -11,6 +11,13 @@ disk_list GET /v1/disks disk_metrics_list GET /v1/disks/{disk}/metrics/{metric} disk_view GET /v1/disks/{disk} +API operations found with tag "floating-ips" +OPERATION ID METHOD URL PATH +floating_ip_create POST /v1/floating-ips +floating_ip_delete DELETE /v1/floating-ips/{floating_ip} +floating_ip_list GET /v1/floating-ips +floating_ip_view GET /v1/floating-ips/{floating_ip} + API operations found with tag "hidden" OPERATION ID METHOD URL PATH device_access_token POST /device/token diff --git a/openapi/nexus.json b/openapi/nexus.json index 0d19e81d9a8..cc105e02c95 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -853,6 +853,204 @@ } } }, + "/v1/floating-ips": { + "get": { + "tags": [ + "floating-ips" + ], + "summary": "List all Floating IPs", + "operationId": "floating_ip_list", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + }, + { + "in": "query", + "name": "project", + "description": "Name or ID of the project", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/NameOrIdSortMode" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ExternalIpResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": { + "required": [ + "project" + ] + } + }, + "post": { + "tags": [ + "floating-ips" + ], + "summary": "Create a Floating IP", + "operationId": "floating_ip_create", + "parameters": [ + { + "in": "query", + "name": "project", + "description": "Name or ID of the project", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/FloatingIpCreate" + } + } + }, + "required": true + }, + "responses": { + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ExternalIp" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/v1/floating-ips/{floating_ip}": { + "get": { + "tags": [ + "floating-ips" + ], + "summary": "Fetch a floating IP", + "operationId": "floating_ip_view", + "parameters": [ + { + "in": "path", + "name": "floating_ip", + "description": "Name or ID of the Floating IP", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "project", + "description": "Name or ID of the project", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ExternalIp" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + }, + "delete": { + "tags": [ + "floating-ips" + ], + "summary": "Delete a Floating IP", + "operationId": "floating_ip_delete", + "parameters": [ + { + "in": "path", + "name": "floating_ip", + "description": "Name or ID of the Floating IP", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "project", + "description": "Name or ID of the project", + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "responses": { + "204": { + "description": "successful deletion" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/groups": { "get": { "tags": [ @@ -10244,6 +10442,25 @@ "required": [ "type" ] + }, + { + "description": "An IP address providing both inbound and outbound access. The address is an existing Floating IP object assigned to the current project.\n\nThe floating IP must not be in use by another instance or service.", + "type": "object", + "properties": { + "floating_ip": { + "$ref": "#/components/schemas/NameOrId" + }, + "type": { + "type": "string", + "enum": [ + "floating" + ] + } + }, + "required": [ + "floating_ip", + "type" + ] } ] }, @@ -10328,6 +10545,37 @@ "role_name" ] }, + "FloatingIpCreate": { + "description": "Parameters for creating a new floating IP address for instances.", + "type": "object", + "properties": { + "address": { + "nullable": true, + "description": "An IP address to reserve for use as a floating IP. This field is optional if a pool is provided, in which case an address will be automatically chosen from there.", + "type": "string", + "format": "ip" + }, + "description": { + "type": "string" + }, + "name": { + "$ref": "#/components/schemas/Name" + }, + "pool": { + "nullable": true, + "description": "The parent IP pool that a floating IP is pulled from. If combined with an explicit address, then that address must be available in the pool.", + "allOf": [ + { + "$ref": "#/components/schemas/NameOrId" + } + ] + } + }, + "required": [ + "description", + "name" + ] + }, "Group": { "description": "View of a Group", "type": "object", @@ -15032,6 +15280,13 @@ "url": "http://docs.oxide.computer/api/disks" } }, + { + "name": "floating-ips", + "description": "Floating IPs allow a project to allocate well-known IPs to instances.", + "externalDocs": { + "url": "http://docs.oxide.computer/api/floating-ips" + } + }, { "name": "hidden", "description": "TODO operations that will not ship to customers", diff --git a/openapi/sled-agent.json b/openapi/sled-agent.json index ed202ddbdb4..fb9732fefcf 100644 --- a/openapi/sled-agent.json +++ b/openapi/sled-agent.json @@ -4305,18 +4305,23 @@ "$ref": "#/components/schemas/DiskRequest" } }, - "external_ips": { + "ephemeral_ip": { + "nullable": true, "description": "Zero or more external IP addresses (either floating or ephemeral), provided to an instance to allow inbound connectivity.", + "type": "string", + "format": "ip" + }, + "firewall_rules": { "type": "array", "items": { - "type": "string", - "format": "ip" + "$ref": "#/components/schemas/VpcFirewallRule" } }, - "firewall_rules": { + "floating_ips": { "type": "array", "items": { - "$ref": "#/components/schemas/VpcFirewallRule" + "type": "string", + "format": "ip" } }, "nics": { @@ -4335,8 +4340,8 @@ "required": [ "dhcp_config", "disks", - "external_ips", "firewall_rules", + "floating_ips", "nics", "properties", "source_nat" diff --git a/sled-agent/src/instance.rs b/sled-agent/src/instance.rs index a6f022f5f20..77bb718ed5c 100644 --- a/sled-agent/src/instance.rs +++ b/sled-agent/src/instance.rs @@ -208,7 +208,8 @@ struct InstanceInner { // Guest NIC and OPTE port information requested_nics: Vec, source_nat: SourceNatConfig, - external_ips: Vec, + ephemeral_ip: Option, + floating_ips: Vec, firewall_rules: Vec, dhcp_config: DhcpCfg, @@ -665,7 +666,8 @@ impl Instance { port_manager, requested_nics: hardware.nics, source_nat: hardware.source_nat, - external_ips: hardware.external_ips, + ephemeral_ip: hardware.ephemeral_ip, + floating_ips: hardware.floating_ips, firewall_rules: hardware.firewall_rules, dhcp_config, requested_disks: hardware.disks, @@ -877,15 +879,20 @@ impl Instance { // Create OPTE ports for the instance let mut opte_ports = Vec::with_capacity(inner.requested_nics.len()); for nic in inner.requested_nics.iter() { - let (snat, external_ips) = if nic.primary { - (Some(inner.source_nat), &inner.external_ips[..]) + let (snat, ephemeral_ip, floating_ips) = if nic.primary { + ( + Some(inner.source_nat), + inner.ephemeral_ip, + &inner.floating_ips[..], + ) } else { - (None, &[][..]) + (None, None, &[][..]) }; let port = inner.port_manager.create_port( nic, snat, - external_ips, + ephemeral_ip, + floating_ips, &inner.firewall_rules, inner.dhcp_config.clone(), )?; diff --git a/sled-agent/src/params.rs b/sled-agent/src/params.rs index b22bd849750..7ab91e68795 100644 --- a/sled-agent/src/params.rs +++ b/sled-agent/src/params.rs @@ -68,7 +68,8 @@ pub struct InstanceHardware { pub source_nat: SourceNatConfig, /// Zero or more external IP addresses (either floating or ephemeral), /// provided to an instance to allow inbound connectivity. - pub external_ips: Vec, + pub ephemeral_ip: Option, + pub floating_ips: Vec, pub firewall_rules: Vec, pub dhcp_config: DhcpConfig, // TODO: replace `propolis_client::*` with locally-modeled request type diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index b87c91768ba..a0affe30d8c 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -853,8 +853,9 @@ impl ServiceManager { let mut ports = vec![]; for svc in &req.services { + // Non-SNAT IPs are floating IPs with `is_service` true. let external_ip; - let (nic, snat, external_ips) = match &svc.details { + let (nic, snat, floating_ips) = match &svc.details { ServiceType::Nexus { external_ip, nic, .. } => { (nic, None, std::slice::from_ref(external_ip)) } @@ -874,16 +875,25 @@ impl ServiceManager { // config allows outbound access which is enough for // Boundary NTP which needs to come up before Nexus. let port = port_manager - .create_port(nic, snat, external_ips, &[], DhcpCfg::default()) + .create_port( + nic, + snat, + None, + floating_ips, + &[], + DhcpCfg::default(), + ) .map_err(|err| Error::ServicePortCreation { service: svc.details.to_string(), err: Box::new(err), })?; // We also need to update the switch with the NAT mappings + // XXX: need to revisit iff. any services get more than one + // address. let (target_ip, first_port, last_port) = match snat { Some(s) => (s.ip, s.first_port, s.last_port), - None => (external_ips[0], 0, u16::MAX), + None => (floating_ips[0], 0, u16::MAX), }; for dpd_client in &dpd_clients { From 73d9f0d4201cf51566bf9ef69819f70d06738bf1 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Thu, 23 Nov 2023 18:53:47 +0000 Subject: [PATCH 08/27] Better view type for floating IPs --- nexus/db-model/src/external_ip.rs | 43 +++++++++++ nexus/src/app/external_ip.rs | 6 +- nexus/src/external_api/http_entrypoints.rs | 8 +- nexus/types/src/external_api/views.rs | 16 ++++ openapi/nexus.json | 85 +++++++++++++++++++++- 5 files changed, 149 insertions(+), 9 deletions(-) diff --git a/nexus/db-model/src/external_ip.rs b/nexus/db-model/src/external_ip.rs index 583bc5c2a2b..ed3acc4e9da 100644 --- a/nexus/db-model/src/external_ip.rs +++ b/nexus/db-model/src/external_ip.rs @@ -18,6 +18,7 @@ use nexus_types::external_api::shared; use nexus_types::external_api::views; use omicron_common::address::NUM_SOURCE_NAT_PORTS; use omicron_common::api::external::Error; +use omicron_common::api::external::IdentityMetadata; use std::convert::TryFrom; use std::net::IpAddr; use uuid::Uuid; @@ -346,3 +347,45 @@ impl TryFrom for views::ExternalIp { Ok(views::ExternalIp { kind, ip: ip.ip.ip() }) } } + +impl TryFrom for views::FloatingIp { + type Error = Error; + + fn try_from(ip: ExternalIp) -> Result { + if ip.is_service { + return Err(Error::internal_error( + "Service IPs should not be exposed in the API", + )); + } + + let project_id = ip.project_id.ok_or(Error::internal_error( + "database schema guarantees parent project for non-service FIP", + ))?; + + let name = ip + .name + .ok_or(Error::internal_error( + "database schema guarantees ID metadata for non-service FIP", + ))? + .into(); + + let description = ip.description.ok_or(Error::internal_error( + "database schema guarantees ID metadata for non-service FIP", + ))?; + + let identity = IdentityMetadata { + id: ip.id, + name, + description, + time_created: ip.time_created, + time_modified: ip.time_modified, + }; + + Ok(views::FloatingIp { + ip: ip.ip.ip(), + identity, + project_id, + instance_id: ip.parent_id, + }) + } +} diff --git a/nexus/src/app/external_ip.rs b/nexus/src/app/external_ip.rs index 0c3a577fda1..b85e37e9ed3 100644 --- a/nexus/src/app/external_ip.rs +++ b/nexus/src/app/external_ip.rs @@ -5,6 +5,7 @@ //! External IP addresses for instances use crate::external_api::views::ExternalIp; +use crate::external_api::views::FloatingIp; use nexus_db_queries::authz; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::lookup; @@ -46,6 +47,7 @@ impl super::Nexus { &'a self, opctx: &'a OpContext, fip_selector: params::FloatingIpSelector, + // XXX: need lookup typed for ExternalIp/FloatingIp ) -> LookupResult> { match fip_selector { params::FloatingIpSelector { floating_ip: NameOrId::Id(id), project: None } => { @@ -81,7 +83,7 @@ impl super::Nexus { opctx: &OpContext, project_lookup: &lookup::Project<'_>, // pagparams: &PaginatedBy<'_>, - ) -> ListResultVec { + ) -> ListResultVec { let (.., authz_project) = project_lookup.lookup_for(authz::Action::Read).await?; Ok(self @@ -98,7 +100,7 @@ impl super::Nexus { opctx: &OpContext, project_lookup: &lookup::Project<'_>, params: ¶ms::FloatingIpCreate, - ) -> CreateResult { + ) -> CreateResult { let (.., authz_project) = project_lookup.lookup_for(authz::Action::CreateChild).await?; diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index dfa686d4339..65cc41e82b4 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1534,7 +1534,7 @@ async fn ip_pool_service_range_remove( async fn floating_ip_list( rqctx: RequestContext>, query_params: Query>, -) -> Result>, HttpError> { +) -> Result>, HttpError> { let apictx = rqctx.context(); let handler = async { let nexus = &apictx.nexus; @@ -1568,7 +1568,7 @@ async fn floating_ip_create( rqctx: RequestContext>, query_params: Query, floating_params: TypedBody, -) -> Result, HttpError> { +) -> Result, HttpError> { let apictx = rqctx.context(); let nexus = &apictx.nexus; let floating_params = floating_params.into_inner(); @@ -1579,7 +1579,7 @@ async fn floating_ip_create( let ip = nexus .floating_ip_create(&opctx, &project_lookup, &floating_params) .await?; - Ok(HttpResponseCreated(views::ExternalIp::from(ip))) + Ok(HttpResponseCreated(ip)) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } @@ -1625,7 +1625,7 @@ async fn floating_ip_view( rqctx: RequestContext>, path_params: Path, query_params: Query, -) -> Result, HttpError> { +) -> Result, HttpError> { todo!(); // let apictx = rqctx.context(); diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index b34fc7a542c..c58b163d6b4 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -265,6 +265,22 @@ pub struct ExternalIp { pub kind: IpKind, } +/// A Floating IP is a well-known IP address which can be attached +/// and detached from instances. +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub struct FloatingIp { + #[serde(flatten)] + pub identity: IdentityMetadata, + /// The IP address held by this resource. + pub ip: IpAddr, + /// The project this resource exists within. + pub project_id: Uuid, + /// The ID of the instance that this Floating IP is attached to, + /// if it is presently in use. + pub instance_id: Option, +} + // RACKS /// View of an Rack diff --git a/openapi/nexus.json b/openapi/nexus.json index cc105e02c95..1b55b7f580d 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -903,7 +903,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/ExternalIpResultsPage" + "$ref": "#/components/schemas/FloatingIpResultsPage" } } } @@ -954,7 +954,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/ExternalIp" + "$ref": "#/components/schemas/FloatingIp" } } } @@ -1000,7 +1000,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/ExternalIp" + "$ref": "#/components/schemas/FloatingIp" } } } @@ -10545,6 +10545,64 @@ "role_name" ] }, + "FloatingIp": { + "description": "A Floating IP is a well-known IP address which can be attached and detached from instances.", + "type": "object", + "properties": { + "description": { + "description": "human-readable free-form text about a resource", + "type": "string" + }, + "id": { + "description": "unique, immutable, system-controlled identifier for each resource", + "type": "string", + "format": "uuid" + }, + "instance_id": { + "nullable": true, + "description": "The ID of the instance that this Floating IP is attached to, if it is presently in use.", + "type": "string", + "format": "uuid" + }, + "ip": { + "description": "The IP address held by this resource.", + "type": "string", + "format": "ip" + }, + "name": { + "description": "unique, mutable, user-controlled identifier for each resource", + "allOf": [ + { + "$ref": "#/components/schemas/Name" + } + ] + }, + "project_id": { + "description": "The project this resource exists within.", + "type": "string", + "format": "uuid" + }, + "time_created": { + "description": "timestamp when this resource was created", + "type": "string", + "format": "date-time" + }, + "time_modified": { + "description": "timestamp when this resource was last modified", + "type": "string", + "format": "date-time" + } + }, + "required": [ + "description", + "id", + "ip", + "name", + "project_id", + "time_created", + "time_modified" + ] + }, "FloatingIpCreate": { "description": "Parameters for creating a new floating IP address for instances.", "type": "object", @@ -10576,6 +10634,27 @@ "name" ] }, + "FloatingIpResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/FloatingIp" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, "Group": { "description": "View of a Group", "type": "object", From c826bb36db92eaf4054ea2905934eaffc6716cdd Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 24 Nov 2023 16:06:40 +0000 Subject: [PATCH 09/27] Add FloatingIp view, resource handling + authz --- common/src/api/external/mod.rs | 2 +- nexus/db-model/src/external_ip.rs | 50 ++++++++- nexus/db-model/src/schema.rs | 18 +++ nexus/db-queries/src/authz/api_resources.rs | 8 ++ nexus/db-queries/src/authz/oso_generic.rs | 1 + .../src/db/datastore/external_ip.rs | 103 ++++++++++++++++-- nexus/db-queries/src/db/datastore/ip_pool.rs | 10 -- nexus/db-queries/src/db/lookup.rs | 24 ++-- nexus/src/app/external_ip.rs | 43 ++++---- nexus/src/external_api/http_entrypoints.rs | 84 +++++++------- nexus/types/src/external_api/views.rs | 2 +- schema/crdb/14.0.0/up05.sql | 19 ++++ schema/crdb/dbinit.sql | 20 ++++ 13 files changed, 285 insertions(+), 99 deletions(-) create mode 100644 schema/crdb/14.0.0/up05.sql diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index b83fb0ce2ca..7c6140044c1 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -751,7 +751,7 @@ pub enum ResourceType { Zpool, Vmm, Ipv4NatEntry, - // ExternalIp, + FloatingIp, } // IDENTITY METADATA diff --git a/nexus/db-model/src/external_ip.rs b/nexus/db-model/src/external_ip.rs index ed3acc4e9da..e79dabb6120 100644 --- a/nexus/db-model/src/external_ip.rs +++ b/nexus/db-model/src/external_ip.rs @@ -6,11 +6,12 @@ //! services. use crate::impl_enum_type; -use crate::schema::external_ip; +use crate::schema::{external_ip, floating_ip}; use crate::Name; use crate::SqlU16; use chrono::DateTime; use chrono::Utc; +use db_macros::Resource; use diesel::Queryable; use diesel::Selectable; use ipnetwork::IpNetwork; @@ -19,6 +20,7 @@ use nexus_types::external_api::views; use omicron_common::address::NUM_SOURCE_NAT_PORTS; use omicron_common::api::external::Error; use omicron_common::api::external::IdentityMetadata; +use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use std::net::IpAddr; use uuid::Uuid; @@ -74,6 +76,28 @@ pub struct ExternalIp { pub project_id: Option, } +/// A view type constructed from `ExternalIp` used to represent Floating IP +/// objects in user-facing APIs. +/// +/// This View type fills a similar niche to `ProjectImage` etc.: we need to +/// represent identity as non-nullable (ditto for parent project) so as to +/// play nicely with authz and resource APIs. +#[derive( + Queryable, Selectable, Clone, Debug, Resource, Serialize, Deserialize, +)] +#[diesel(table_name = floating_ip)] +pub struct FloatingIp { + #[diesel(embed)] + pub identity: FloatingIpIdentity, + + pub ip_pool_id: Uuid, + pub ip_pool_range_id: Uuid, + pub is_service: bool, + pub parent_id: Option, + pub ip: IpNetwork, + pub project_id: Uuid, +} + impl From for sled_agent_client::types::SourceNatConfig { fn from(eip: ExternalIp) -> Self { Self { @@ -352,6 +376,11 @@ impl TryFrom for views::FloatingIp { type Error = Error; fn try_from(ip: ExternalIp) -> Result { + if ip.kind != IpKind::Floating { + return Err(Error::internal_error( + "attempted to convert non-floating external IP to floating", + )); + } if ip.is_service { return Err(Error::internal_error( "Service IPs should not be exposed in the API", @@ -389,3 +418,22 @@ impl TryFrom for views::FloatingIp { }) } } + +impl From for views::FloatingIp { + fn from(ip: FloatingIp) -> Self { + let identity = IdentityMetadata { + id: ip.identity.id, + name: ip.identity.name.into(), + description: ip.identity.description, + time_created: ip.identity.time_created, + time_modified: ip.identity.time_modified, + }; + + views::FloatingIp { + ip: ip.ip.ip(), + identity, + project_id: ip.project_id, + instance_id: ip.parent_id, + } + } +} diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index 1449c3d20eb..90ab6773bcb 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -538,6 +538,24 @@ table! { } } +table! { + floating_ip (id) { + id -> Uuid, + name -> Text, + description -> Text, + time_created -> Timestamptz, + time_modified -> Timestamptz, + time_deleted -> Nullable, + + ip_pool_id -> Uuid, + ip_pool_range_id -> Uuid, + is_service -> Bool, + parent_id -> Nullable, + ip -> Inet, + project_id -> Uuid, + } +} + table! { silo (id) { id -> Uuid, diff --git a/nexus/db-queries/src/authz/api_resources.rs b/nexus/db-queries/src/authz/api_resources.rs index b22fe1ac25d..2dfe2f71745 100644 --- a/nexus/db-queries/src/authz/api_resources.rs +++ b/nexus/db-queries/src/authz/api_resources.rs @@ -791,6 +791,14 @@ authz_resource! { polar_snippet = InProject, } +authz_resource! { + name = "FloatingIp", + parent = "Project", + primary_key = Uuid, + roles_allowed = false, + polar_snippet = InProject, +} + // Customer network integration resources nested below "Fleet" authz_resource! { diff --git a/nexus/db-queries/src/authz/oso_generic.rs b/nexus/db-queries/src/authz/oso_generic.rs index e642062eada..60983792879 100644 --- a/nexus/db-queries/src/authz/oso_generic.rs +++ b/nexus/db-queries/src/authz/oso_generic.rs @@ -131,6 +131,7 @@ pub fn make_omicron_oso(log: &slog::Logger) -> Result { VpcRouter::init(), RouterRoute::init(), VpcSubnet::init(), + FloatingIp::init(), // Silo-level resources Image::init(), SiloImage::init(), diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index 4fe0c6033ad..9ac72705a83 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -13,9 +13,11 @@ use crate::db::error::public_error_from_diesel; use crate::db::error::ErrorHandler; use crate::db::lookup::LookupPath; use crate::db::model::ExternalIp; +use crate::db::model::FloatingIp; use crate::db::model::IncompleteExternalIp; use crate::db::model::IpKind; use crate::db::model::Name; +use crate::db::pagination::paginated; use crate::db::pool::DbConnection; use crate::db::queries::external_ip::NextExternalIp; use crate::db::update_and_check::UpdateAndCheck; @@ -24,11 +26,15 @@ use async_bb8_diesel::AsyncRunQueryDsl; use chrono::Utc; use diesel::prelude::*; use nexus_types::identity::Resource; +use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_common::api::external::CreateResult; +use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; +use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::NameOrId; use omicron_common::api::external::UpdateResult; +use ref_cast::RefCast; use std::net::IpAddr; use uuid::Uuid; @@ -344,8 +350,6 @@ impl DataStore { /// This method returns the number of records deleted, rather than the usual /// `DeleteResult`. That's mostly useful for tests, but could be important /// if callers have some invariants they'd like to check. - // TODO-correctness: This can't be used for Floating IPs, we'll need a - // _detatch_ method for that. pub async fn deallocate_external_ip_by_instance_id( &self, opctx: &OpContext, @@ -364,6 +368,27 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } + /// Detach an individual Floating IP address from its parent instance. + /// + /// As in `deallocate_external_ip_by_instance_id`, This method returns the + /// number of records deleted, rather than the usual `DeleteResult`. + pub async fn detach_floating_ips_by_instance_id( + &self, + opctx: &OpContext, + instance_id: Uuid, + ) -> Result { + use db::schema::external_ip::dsl; + diesel::update(dsl::external_ip) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::is_service.eq(false)) + .filter(dsl::parent_id.eq(instance_id)) + .filter(dsl::kind.eq(IpKind::Floating)) + .set(dsl::parent_id.eq(Option::::None)) + .execute_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + /// Fetch all external IP addresses of any kind for the provided instance pub async fn instance_lookup_external_ips( &self, @@ -381,20 +406,74 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } - /// Fetch all floating IP addresses for the provided project. - pub async fn lookup_floating_ips( + /// Fetch all Floating IP addresses for the provided project. + pub async fn floating_ips_list( &self, opctx: &OpContext, - project_id: Uuid, - ) -> LookupResult> { + authz_project: &authz::Project, + pagparams: &PaginatedBy<'_>, + ) -> ListResultVec { + opctx.authorize(authz::Action::ListChildren, authz_project).await?; + + use db::schema::floating_ip::dsl; + + match pagparams { + PaginatedBy::Id(pagparams) => { + paginated(dsl::floating_ip, dsl::id, &pagparams) + } + PaginatedBy::Name(pagparams) => paginated( + dsl::floating_ip, + dsl::name, + &pagparams.map_name(|n| Name::ref_cast(n)), + ), + } + .filter(dsl::project_id.eq(authz_project.id())) + .filter(dsl::time_deleted.is_null()) + .select(FloatingIp::as_select()) + .get_results_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + + /// Delete a Floating IP, verifying first that it is not in use. + pub async fn floating_ip_delete( + &self, + opctx: &OpContext, + authz_fip: &authz::FloatingIp, + db_fip: &FloatingIp, + ) -> DeleteResult { use db::schema::external_ip::dsl; - dsl::external_ip - .filter(dsl::project_id.eq(project_id)) + + // Verify this FIP is not attached to any instances/services. + if db_fip.parent_id.is_some() { + return Err(Error::invalid_request( + "Floating IP cannot be deleted while attached to an instance", + )); + } + + opctx.authorize(authz::Action::Delete, authz_fip).await?; + + let now = Utc::now(); + let updated_rows = diesel::update(dsl::external_ip) + .filter(dsl::id.eq(db_fip.id())) .filter(dsl::time_deleted.is_null()) - .filter(dsl::kind.eq(IpKind::Floating)) - .select(ExternalIp::as_select()) - .get_results_async(&*self.pool_connection_authorized(opctx).await?) + .filter(dsl::parent_id.is_null()) + .set(dsl::time_deleted.eq(now)) + .execute_async(&*self.pool_connection_authorized(opctx).await?) .await - .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource(authz_fip), + ) + })?; + + if updated_rows == 0 { + return Err(Error::InvalidRequest { + message: "deletion failed due to concurrent modification" + .to_string(), + }); + } + Ok(()) } } diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index 6be134fd652..fb300ef8338 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -110,16 +110,6 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) } - /// Lookup an IP pool within the current silo which contains a target IP - /// address. - pub async fn ip_pools_fetch_for_ip( - &self, - opctx: &OpContext, - ip_addr: std::net::IpAddr, - ) -> LookupResult { - todo!() - } - /// Looks up an IP pool intended for internal services. /// /// This method may require an index by Availability Zone in the future. diff --git a/nexus/db-queries/src/db/lookup.rs b/nexus/db-queries/src/db/lookup.rs index ead2350a506..028694dc4b8 100644 --- a/nexus/db-queries/src/db/lookup.rs +++ b/nexus/db-queries/src/db/lookup.rs @@ -231,6 +231,11 @@ impl<'a> LookupPath<'a> { RouterRoute::PrimaryKey(Root { lookup_root: self }, id) } + /// Select a resource of type FloatingIp, identified by its id + pub fn floating_ip_id(self, id: Uuid) -> FloatingIp<'a> { + FloatingIp::PrimaryKey(Root { lookup_root: self }, id) + } + // Fleet-level resources /// Select a resource of type ConsoleSession, identified by its `token` @@ -632,8 +637,7 @@ lookup_resource! { lookup_resource! { name = "Project", ancestors = [ "Silo" ], - // children = [ "Disk", "Instance", "Vpc", "Snapshot", "ProjectImage", "ExternalIp" ], - children = [ "Disk", "Instance", "Vpc", "Snapshot", "ProjectImage" ], + children = [ "Disk", "Instance", "Vpc", "Snapshot", "ProjectImage", "FloatingIp" ], lookup_by_name = true, soft_deletes = true, primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] @@ -729,14 +733,14 @@ lookup_resource! { primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] } -// lookup_resource! { -// name = "ExternalIp", -// ancestors = [ "Silo", "Project" ], -// children = [], -// lookup_by_name = true, -// soft_deletes = true, -// primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] -// } +lookup_resource! { + name = "FloatingIp", + ancestors = [ "Silo", "Project" ], + children = [], + lookup_by_name = true, + soft_deletes = true, + primary_key_columns = [ { column_name = "id", rust_type = Uuid } ] +} // Miscellaneous resources nested directly below "Fleet" diff --git a/nexus/src/app/external_ip.rs b/nexus/src/app/external_ip.rs index b85e37e9ed3..6cc2d9f171a 100644 --- a/nexus/src/app/external_ip.rs +++ b/nexus/src/app/external_ip.rs @@ -12,6 +12,7 @@ use nexus_db_queries::db::lookup; use nexus_db_queries::db::lookup::LookupPath; use nexus_db_queries::db::model::IpKind; use nexus_types::external_api::params; +use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_common::api::external::CreateResult; use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; @@ -43,28 +44,25 @@ impl super::Nexus { .collect::>()) } - pub fn floating_ip_lookup<'a>( + pub(crate) fn floating_ip_lookup<'a>( &'a self, opctx: &'a OpContext, fip_selector: params::FloatingIpSelector, - // XXX: need lookup typed for ExternalIp/FloatingIp - ) -> LookupResult> { + ) -> LookupResult> { match fip_selector { params::FloatingIpSelector { floating_ip: NameOrId::Id(id), project: None } => { - // let floating_ip = - // LookupPath::new(opctx, &self.db_datastore).floating_ip_id(id); - // Ok(floating_ip) - todo!() + let floating_ip = + LookupPath::new(opctx, &self.db_datastore).floating_ip_id(id); + Ok(floating_ip) } params::FloatingIpSelector { floating_ip: NameOrId::Name(name), project: Some(project), } => { - // let floating_ip = self - // .project_lookup(opctx, params::ProjectSelector { project })? - // .floating_ip_name_owned(name.into()); - // Ok(floating_ip) - todo!() + let floating_ip = self + .project_lookup(opctx, params::ProjectSelector { project })? + .floating_ip_name_owned(name.into()); + Ok(floating_ip) } params::FloatingIpSelector { floating_ip: NameOrId::Id(_), @@ -82,17 +80,18 @@ impl super::Nexus { &self, opctx: &OpContext, project_lookup: &lookup::Project<'_>, - // pagparams: &PaginatedBy<'_>, + pagparams: &PaginatedBy<'_>, ) -> ListResultVec { let (.., authz_project) = - project_lookup.lookup_for(authz::Action::Read).await?; + project_lookup.lookup_for(authz::Action::ListChildren).await?; + Ok(self .db_datastore - .lookup_floating_ips(opctx, authz_project.id()) + .floating_ips_list(opctx, &authz_project, pagparams) .await? .into_iter() - .map(|ip| ip.try_into().unwrap()) - .collect::>()) + .map(Into::into) + .collect()) } pub(crate) async fn floating_ip_create( @@ -134,11 +133,11 @@ impl super::Nexus { pub(crate) async fn floating_ip_delete( &self, opctx: &OpContext, - // pool_lookup: &lookup::<'_>, + ip_lookup: lookup::FloatingIp<'_>, ) -> DeleteResult { - // let (.., authz_pool, db_pool) = - // pool_lookup.fetch_for(authz::Action::Delete).await?; - // self.db_datastore.ip_pool_delete(opctx, &authz_pool, &db_pool).await - todo!() + let (.., authz_fip, db_fip) = + ip_lookup.fetch_for(authz::Action::Delete).await?; + + self.db_datastore.floating_ip_delete(opctx, &authz_fip, &db_fip).await } } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 65cc41e82b4..9081163e221 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1545,15 +1545,14 @@ async fn floating_ip_list( let paginated_by = name_or_id_pagination(&pag_params, scan_params)?; let project_lookup = nexus.project_lookup(&opctx, scan_params.selector.clone())?; - // XXX: impl relevant traits to make results_page work. - // let ips = nexus.list_floating_ips(&opctx, &project_lookup, &paginated_by).await?; - let ips = nexus.floating_ips_list(&opctx, &project_lookup).await?; - Ok(HttpResponseOk(ResultsPage { items: ips, next_page: None })) - // Ok(HttpResponseOk(ScanByNameOrId::results_page( - // &query, - // ips, - // &marker_for_name_or_id, - // )?)) + let ips = nexus + .floating_ips_list(&opctx, &project_lookup, &paginated_by) + .await?; + Ok(HttpResponseOk(ScanByNameOrId::results_page( + &query, + ips, + &marker_for_name_or_id, + )?)) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } @@ -1595,24 +1594,23 @@ async fn floating_ip_delete( path_params: Path, query_params: Query, ) -> Result { - todo!(); - - // TODO: more work needed here to plug into authz, and pathlookup - // etc. - - // let apictx = rqctx.context(); - // let handler = async { - // let opctx = crate::context::op_context_for_external_api(&rqctx).await?; - // let nexus = &apictx.nexus; - // let path = path_params.into_inner(); - // let query = query_params.into_inner(); - // let disk_selector = - // params::FloatingIpSelector { floating_ip: path.floating_ip, project: query.project }; - // let disk_lookup = nexus.disk_lookup(&opctx, disk_selector)?; - // nexus.project_delete_disk(&opctx, &disk_lookup).await?; - // Ok(HttpResponseDeleted()) - // }; - // apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await + let apictx = rqctx.context(); + let handler = async { + let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let query = query_params.into_inner(); + let floating_ip_selector = params::FloatingIpSelector { + floating_ip: path.floating_ip, + project: query.project, + }; + let fip_lookup = + nexus.floating_ip_lookup(&opctx, floating_ip_selector)?; + + nexus.floating_ip_delete(&opctx, fip_lookup).await?; + Ok(HttpResponseDeleted()) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } /// Fetch a floating IP @@ -1626,21 +1624,23 @@ async fn floating_ip_view( path_params: Path, query_params: Query, ) -> Result, HttpError> { - todo!(); - - // let apictx = rqctx.context(); - // let handler = async { - // let opctx = crate::context::op_context_for_external_api(&rqctx).await?; - // let nexus = &apictx.nexus; - // let path = path_params.into_inner(); - // let query = query_params.into_inner(); - // let disk_selector = - // params::FloatingIpSelector { floating_ip: path.floating_ip, project: query.project }; - // let (.., disk) = - // nexus.disk_lookup(&opctx, disk_selector)?.fetch().await?; - // Ok(HttpResponseOk(disk.into())) - // }; - // apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await + let apictx = rqctx.context(); + let handler = async { + let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let query = query_params.into_inner(); + let floating_ip_selector = params::FloatingIpSelector { + floating_ip: path.floating_ip, + project: query.project, + }; + let (.., fip) = nexus + .floating_ip_lookup(&opctx, floating_ip_selector)? + .fetch() + .await?; + Ok(HttpResponseOk(fip.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } // Disks diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index c58b163d6b4..286ec0ef40b 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -267,7 +267,7 @@ pub struct ExternalIp { /// A Floating IP is a well-known IP address which can be attached /// and detached from instances. -#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] +#[derive(ObjectIdentity, Debug, Clone, Deserialize, Serialize, JsonSchema)] #[serde(rename_all = "snake_case")] pub struct FloatingIp { #[serde(flatten)] diff --git a/schema/crdb/14.0.0/up05.sql b/schema/crdb/14.0.0/up05.sql new file mode 100644 index 00000000000..3e172e3e705 --- /dev/null +++ b/schema/crdb/14.0.0/up05.sql @@ -0,0 +1,19 @@ +CREATE VIEW IF NOT EXISTS omicron.public.floating_ip AS +SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + ip_pool_id, + ip_pool_range_id, + is_service, + parent_id, + ip, + project_id +FROM + omicron.public.external_ip +WHERE + omicron.public.external_ip.kind = 'floating' AND + project_id IS NOT NULL; diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 1c31bcc31b9..2161adb7c52 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -1692,6 +1692,26 @@ CREATE UNIQUE INDEX IF NOT EXISTS lookup_floating_ip_by_name_and_project on omic time_deleted is NULL AND project_id is NOT NULL; +CREATE VIEW IF NOT EXISTS omicron.public.floating_ip AS +SELECT + id, + name, + description, + time_created, + time_modified, + time_deleted, + ip_pool_id, + ip_pool_range_id, + is_service, + parent_id, + ip, + project_id +FROM + omicron.public.external_ip +WHERE + omicron.public.external_ip.kind = 'floating' AND + project_id IS NOT NULL; + /*******************************************************************/ /* From dd37e41fa888d6f5b5b06b37f37407457242dd25 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 24 Nov 2023 17:57:58 +0000 Subject: [PATCH 10/27] Create/Delete-time Floating IP management --- nexus/db-model/src/external_ip.rs | 22 ++- .../src/db/datastore/external_ip.rs | 165 ++++++++++++------ nexus/src/app/sagas/instance_create.rs | 40 +++-- nexus/src/app/sagas/instance_delete.rs | 5 + nexus/types/src/external_api/params.rs | 2 +- openapi/nexus.json | 6 +- 6 files changed, 160 insertions(+), 80 deletions(-) diff --git a/nexus/db-model/src/external_ip.rs b/nexus/db-model/src/external_ip.rs index e79dabb6120..40ad3e2b923 100644 --- a/nexus/db-model/src/external_ip.rs +++ b/nexus/db-model/src/external_ip.rs @@ -372,7 +372,7 @@ impl TryFrom for views::ExternalIp { } } -impl TryFrom for views::FloatingIp { +impl TryFrom for FloatingIp { type Error = Error; fn try_from(ip: ExternalIp) -> Result { @@ -402,23 +402,35 @@ impl TryFrom for views::FloatingIp { "database schema guarantees ID metadata for non-service FIP", ))?; - let identity = IdentityMetadata { + let identity = FloatingIpIdentity { id: ip.id, name, description, time_created: ip.time_created, time_modified: ip.time_modified, + time_deleted: ip.time_deleted, }; - Ok(views::FloatingIp { - ip: ip.ip.ip(), + Ok(FloatingIp { + ip: ip.ip, identity, project_id, - instance_id: ip.parent_id, + ip_pool_id: ip.ip_pool_id, + ip_pool_range_id: ip.ip_pool_range_id, + is_service: ip.is_service, + parent_id: ip.parent_id, }) } } +impl TryFrom for views::FloatingIp { + type Error = Error; + + fn try_from(ip: ExternalIp) -> Result { + FloatingIp::try_from(ip).map(Into::into) + } +} + impl From for views::FloatingIp { fn from(ip: FloatingIp) -> Self { let identity = IdentityMetadata { diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index 9ac72705a83..a624e5f5b76 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -32,7 +32,7 @@ use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; -use omicron_common::api::external::NameOrId; +use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; use ref_cast::RefCast; use std::net::IpAddr; @@ -153,8 +153,6 @@ impl DataStore { self.ip_pools_fetch_default(opctx).await?.id() }; - // XXX: Verify that chosen pool comes from my silo. - let data = if let Some(ip) = ip { IncompleteExternalIp::for_floating_explicit( ip_id, @@ -174,61 +172,7 @@ impl DataStore { ) }; - // TODO: need to disambiguate no IP and/or IP taken - // from resource name collision, and expose those in - // a nice way. self.allocate_external_ip(opctx, data).await - // .map_err(|e| { - // public_error_from_diesel( - // e, - // ErrorHandler::Conflict(todo!(), name.as_str()) - // ) - // }) - } - - /// Allocates a floating IP address for instance usage. - pub async fn attach_floating_ip_to_instance( - &self, - opctx: &OpContext, - _instance_id: Uuid, - _ip_id: &NameOrId, - _project: &authz::Project, - ) -> UpdateResult { - // use db::schema::external_ip::dsl; - // TODO: scope by project - // opctx.authorize(authz::Action::CreateChild, authz_project).await?; - let _conn = self.pool_connection_authorized(opctx).await?; - - // let ip_id = match ip_id { - // NameOrId::Id(id) => *id, - // NameOrId::Name(name) => { - // diesel::select(dsl::external_ip) - // .filter(dsl::time_deleted.is_null()) - // .filter(dsl::name.eq(&name)) - // .execute_and_check(&conn) - // .await - // .map(|m| m.) - // } - // }; - - // opctx.authn. - // todo!() - - // diesel::update(dsl::external_ip) - // .filter(dsl::time_deleted.is_null()) - // .filter(dsl::id.eq(Some(ip_id))) - // .filter(dsl::parent_id.is_null()) - // .set(dsl::parent_id.eq(instance_id)) - // .check_if_exists::(ip_id) - // .execute_and_check(&*self.pool_connection_authorized(opctx).await?) - // .await - // .map(|r| match r.status { - // UpdateStatus::Updated => true, - // UpdateStatus::NotUpdatedButExists => false, - // }) - // .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) - - todo!() } async fn allocate_external_ip( @@ -246,8 +190,12 @@ impl DataStore { conn: &async_bb8_diesel::Connection, data: IncompleteExternalIp, ) -> CreateResult { + // Name needs to be cloned out here (if present) to give users a + // sensible error message on name collision. + let name = data.name().clone(); let explicit_ip = data.explicit_ip().is_some(); NextExternalIp::new(data).get_result_async(conn).await.map_err(|e| { + use diesel::result::Error::DatabaseError; use diesel::result::Error::NotFound; match e { NotFound => { @@ -261,6 +209,17 @@ impl DataStore { ) } } + DatabaseError(..) if name.is_some() => { + public_error_from_diesel( + e, + ErrorHandler::Conflict( + ResourceType::FloatingIp, + name.as_ref() + .map(|m| m.as_str()) + .unwrap_or_default(), + ), + ) + } _ => crate::db::queries::external_ip::from_diesel(e), } }) @@ -476,4 +435,96 @@ impl DataStore { } Ok(()) } + + /// Attaches a Floating IP address to an instance. + pub async fn floating_ip_attach( + &self, + opctx: &OpContext, + authz_fip: &authz::FloatingIp, + db_fip: &FloatingIp, + instance_id: Uuid, + ) -> UpdateResult { + use db::schema::external_ip::dsl; + + // Verify this FIP is not attached to any instances/services. + if db_fip.parent_id.is_some() { + return Err(Error::invalid_request( + "Floating IP cannot be attached to one instance while still attached to another", + )); + } + + let (.., authz_instance, _db_instance) = LookupPath::new(&opctx, self) + .instance_id(instance_id) + .fetch_for(authz::Action::Modify) + .await?; + + opctx.authorize(authz::Action::Modify, authz_fip).await?; + opctx.authorize(authz::Action::Modify, &authz_instance).await?; + + diesel::update(dsl::external_ip) + .filter(dsl::id.eq(db_fip.id())) + .filter(dsl::kind.eq(IpKind::Floating)) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::parent_id.is_null()) + .set(( + dsl::parent_id.eq(Some(instance_id)), + dsl::time_modified.eq(Utc::now()), + )) + .returning(ExternalIp::as_returning()) + .get_result_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource(authz_fip), + ) + }) + .and_then(|r| FloatingIp::try_from(r)) + .map_err(|e| Error::internal_error(&format!("{e}"))) + } + + /// Detaches a Floating IP address from an instance. + pub async fn floating_ip_detach( + &self, + opctx: &OpContext, + authz_fip: &authz::FloatingIp, + db_fip: &FloatingIp, + ) -> UpdateResult { + use db::schema::external_ip::dsl; + + let Some(instance_id) = db_fip.parent_id else { + return Err(Error::invalid_request( + "Floating IP is not attached to an instance", + )); + }; + + let (.., authz_instance, _db_instance) = LookupPath::new(&opctx, self) + .instance_id(instance_id) + .fetch_for(authz::Action::Modify) + .await?; + + opctx.authorize(authz::Action::Modify, authz_fip).await?; + opctx.authorize(authz::Action::Modify, &authz_instance).await?; + + diesel::update(dsl::external_ip) + .filter(dsl::id.eq(db_fip.id())) + .filter(dsl::kind.eq(IpKind::Floating)) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::parent_id.eq(instance_id)) + .set(( + dsl::parent_id.eq(Option::::None), + dsl::time_modified.eq(Utc::now()), + )) + .returning(ExternalIp::as_returning()) + .get_result_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource(authz_fip), + ) + }) + .and_then(|r| FloatingIp::try_from(r)) + .map_err(|e| Error::internal_error(&format!("{e}"))) + } } diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs index 87d9366780c..e1c4f273a72 100644 --- a/nexus/src/app/sagas/instance_create.rs +++ b/nexus/src/app/sagas/instance_create.rs @@ -620,23 +620,35 @@ async fn sic_allocate_instance_external_ip( let ip_id = repeat_saga_params.new_id; // Collect the possible pool name for this IP address - let pool_name = match ip_params { + match ip_params { params::ExternalIpCreate::Ephemeral { ref pool_name } => { - pool_name.as_ref().map(|name| db::model::Name(name.clone())) + let pool_name = + pool_name.as_ref().map(|name| db::model::Name(name.clone())); + datastore + .allocate_instance_ephemeral_ip( + &opctx, + ip_id, + instance_id, + pool_name, + ) + .await + .map_err(ActionError::action_failed)?; } - params::ExternalIpCreate::Floating { ref floating_ip } => { - // In floating case, need: - // floating IP does not belong to another instance - // floating IP belongs to the parent project. - return Err(ActionError::action_failed(format!( - "can't yet bind floating ip {floating_ip:?} to instance" - ))); + params::ExternalIpCreate::Floating { ref floating_ip_name } => { + let floating_ip_name = db::model::Name(floating_ip_name.clone()); + let (.., authz_fip, db_fip) = LookupPath::new(&opctx, &datastore) + .project_id(saga_params.project_id) + .floating_ip_name(&floating_ip_name) + .fetch_for(authz::Action::Modify) + .await + .map_err(ActionError::action_failed)?; + + datastore + .floating_ip_attach(&opctx, &authz_fip, &db_fip, instance_id) + .await + .map_err(ActionError::action_failed)?; } - }; - datastore - .allocate_instance_ephemeral_ip(&opctx, ip_id, instance_id, pool_name) - .await - .map_err(ActionError::action_failed)?; + } Ok(()) } diff --git a/nexus/src/app/sagas/instance_delete.rs b/nexus/src/app/sagas/instance_delete.rs index 1605465c74e..4160bcaea21 100644 --- a/nexus/src/app/sagas/instance_delete.rs +++ b/nexus/src/app/sagas/instance_delete.rs @@ -127,6 +127,11 @@ async fn sid_deallocate_external_ip( ) .await .map_err(ActionError::action_failed)?; + osagactx + .datastore() + .detach_floating_ips_by_instance_id(&opctx, params.authz_instance.id()) + .await + .map_err(ActionError::action_failed)?; Ok(()) } diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 0c24570e570..12dd01299b3 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -851,7 +851,7 @@ pub enum ExternalIpCreate { /// an existing Floating IP object assigned to the current project. /// /// The floating IP must not be in use by another instance or service. - Floating { floating_ip: NameOrId }, + Floating { floating_ip_name: Name }, } /// Create-time parameters for an `Instance` diff --git a/openapi/nexus.json b/openapi/nexus.json index 1b55b7f580d..cb45018a99d 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10447,8 +10447,8 @@ "description": "An IP address providing both inbound and outbound access. The address is an existing Floating IP object assigned to the current project.\n\nThe floating IP must not be in use by another instance or service.", "type": "object", "properties": { - "floating_ip": { - "$ref": "#/components/schemas/NameOrId" + "floating_ip_name": { + "$ref": "#/components/schemas/Name" }, "type": { "type": "string", @@ -10458,7 +10458,7 @@ } }, "required": [ - "floating_ip", + "floating_ip_name", "type" ] } From 649cce50ce38befc3f8ee0e23dcd6725a5bae98d Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 24 Nov 2023 18:53:50 +0000 Subject: [PATCH 11/27] Appeasement for Clippy --- nexus/db-model/src/external_ip.rs | 9 ++-- .../src/db/datastore/external_ip.rs | 43 ++++++++++++------- nexus/src/app/external_ip.rs | 24 +---------- nexus/src/external_api/http_entrypoints.rs | 2 +- 4 files changed, 34 insertions(+), 44 deletions(-) diff --git a/nexus/db-model/src/external_ip.rs b/nexus/db-model/src/external_ip.rs index 40ad3e2b923..bbaa8045dad 100644 --- a/nexus/db-model/src/external_ip.rs +++ b/nexus/db-model/src/external_ip.rs @@ -391,12 +391,9 @@ impl TryFrom for FloatingIp { "database schema guarantees parent project for non-service FIP", ))?; - let name = ip - .name - .ok_or(Error::internal_error( - "database schema guarantees ID metadata for non-service FIP", - ))? - .into(); + let name = ip.name.ok_or(Error::internal_error( + "database schema guarantees ID metadata for non-service FIP", + ))?; let description = ip.description.ok_or(Error::internal_error( "database schema guarantees ID metadata for non-service FIP", diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index a624e5f5b76..ccee30726e3 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -25,6 +25,7 @@ use crate::db::update_and_check::UpdateStatus; use async_bb8_diesel::AsyncRunQueryDsl; use chrono::Utc; use diesel::prelude::*; +use nexus_types::external_api::params; use nexus_types::identity::Resource; use omicron_common::api::external::http_pagination::PaginatedBy; use omicron_common::api::external::CreateResult; @@ -32,6 +33,7 @@ use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; +use omicron_common::api::external::NameOrId; use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; use ref_cast::RefCast; @@ -138,26 +140,37 @@ impl DataStore { pub async fn allocate_floating_ip( &self, opctx: &OpContext, - pool_id: Option, project_id: Uuid, - ip_id: Uuid, - name: &Name, - description: &str, - ip: Option, + params: params::FloatingIpCreate, ) -> CreateResult { + let ip_id = Uuid::new_v4(); + // XXX: mux here to scan *all* project pools in // current silo for convenience? - let pool_id = if let Some(id) = pool_id { - id - } else { - self.ip_pools_fetch_default(opctx).await?.id() - }; + let pool_id = match params.pool { + Some(NameOrId::Name(name)) => { + LookupPath::new(opctx, self) + .ip_pool_name(&Name(name)) + .fetch_for(authz::Action::Read) + .await? + .1 + } + Some(NameOrId::Id(id)) => { + LookupPath::new(opctx, self) + .ip_pool_id(id) + .fetch_for(authz::Action::Read) + .await? + .1 + } + None => self.ip_pools_fetch_default(opctx).await?, + } + .id(); - let data = if let Some(ip) = ip { + let data = if let Some(ip) = params.address { IncompleteExternalIp::for_floating_explicit( ip_id, - name, - description, + &Name(params.identity.name), + ¶ms.identity.description, project_id, ip, pool_id, @@ -165,8 +178,8 @@ impl DataStore { } else { IncompleteExternalIp::for_floating( ip_id, - name, - description, + &Name(params.identity.name), + ¶ms.identity.description, project_id, pool_id, ) diff --git a/nexus/src/app/external_ip.rs b/nexus/src/app/external_ip.rs index 6cc2d9f171a..404f5972888 100644 --- a/nexus/src/app/external_ip.rs +++ b/nexus/src/app/external_ip.rs @@ -19,7 +19,6 @@ use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; use omicron_common::api::external::NameOrId; -use uuid::Uuid; impl super::Nexus { pub(crate) async fn instance_list_external_ips( @@ -98,33 +97,14 @@ impl super::Nexus { &self, opctx: &OpContext, project_lookup: &lookup::Project<'_>, - params: ¶ms::FloatingIpCreate, + params: params::FloatingIpCreate, ) -> CreateResult { let (.., authz_project) = project_lookup.lookup_for(authz::Action::CreateChild).await?; - // XXX: support pool by name here. - let pool_id = match ¶ms.pool { - Some(NameOrId::Id(ref id)) => Some(*id), - Some(NameOrId::Name(ref _name)) => { - return Err(Error::internal_error( - "pool ref by name not yet supported", - )) - } - None => None, - }; - Ok(self .db_datastore - .allocate_floating_ip( - opctx, - pool_id, - authz_project.id(), - Uuid::new_v4(), - ¶ms.identity.name.clone().into(), - ¶ms.identity.description, - params.address, - ) + .allocate_floating_ip(opctx, authz_project.id(), params) .await? .try_into() .unwrap()) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 9081163e221..9b90417d5e1 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1576,7 +1576,7 @@ async fn floating_ip_create( let project_lookup = nexus.project_lookup(&opctx, query_params.into_inner())?; let ip = nexus - .floating_ip_create(&opctx, &project_lookup, &floating_params) + .floating_ip_create(&opctx, &project_lookup, floating_params) .await?; Ok(HttpResponseCreated(ip)) }; From 57620d81190cc78683047faa1beb77b64d943cca Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 24 Nov 2023 18:57:51 +0000 Subject: [PATCH 12/27] Cargo hakari + OPTE false version --- Cargo.lock | 1 + tools/opte_version | 2 +- workspace-hack/Cargo.toml | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 3c851e093bc..78f3fb9d017 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4966,6 +4966,7 @@ dependencies = [ "clap_builder", "console", "const-oid", + "crc32fast", "crossbeam-epoch", "crossbeam-utils", "crossterm", diff --git a/tools/opte_version b/tools/opte_version index 0a79a6aba9b..82d79dcf282 100644 --- a/tools/opte_version +++ b/tools/opte_version @@ -1 +1 @@ -0.25.183 +0.27.214 diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index 1a289bd0cbb..6ad4ed78bab 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -29,6 +29,7 @@ clap = { version = "4.4.3", features = ["derive", "env", "wrap_help"] } clap_builder = { version = "4.4.2", default-features = false, features = ["color", "env", "std", "suggestions", "usage", "wrap_help"] } console = { version = "0.15.7" } const-oid = { version = "0.9.5", default-features = false, features = ["db", "std"] } +crc32fast = { version = "1.3.2" } crossbeam-epoch = { version = "0.9.15" } crossbeam-utils = { version = "0.8.16" } crossterm = { version = "0.27.0", features = ["event-stream", "serde"] } @@ -124,6 +125,7 @@ clap = { version = "4.4.3", features = ["derive", "env", "wrap_help"] } clap_builder = { version = "4.4.2", default-features = false, features = ["color", "env", "std", "suggestions", "usage", "wrap_help"] } console = { version = "0.15.7" } const-oid = { version = "0.9.5", default-features = false, features = ["db", "std"] } +crc32fast = { version = "1.3.2" } crossbeam-epoch = { version = "0.9.15" } crossbeam-utils = { version = "0.8.16" } crossterm = { version = "0.27.0", features = ["event-stream", "serde"] } From 0cdbc1cf8c25f27b821d4afd4de4ae38dbae7f7e Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Sat, 25 Nov 2023 00:57:37 +0000 Subject: [PATCH 13/27] Apply missed schema version bump --- schema/crdb/dbinit.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 2161adb7c52..b890dd19f8a 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -3007,7 +3007,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - ( TRUE, NOW(), NOW(), '13.0.0', NULL) + ( TRUE, NOW(), NOW(), '14.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; From 520e58916b44124cc2af5c131521bba41cfef09d Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 27 Nov 2023 13:41:54 +0000 Subject: [PATCH 14/27] Fix External IP dup-name error checking, constraints test --- Cargo.lock | 16 +- Cargo.toml | 4 +- .../src/db/datastore/external_ip.rs | 3 +- nexus/db-queries/src/db/datastore/mod.rs | 310 ++++++++++-------- schema/crdb/14.0.0/up06.sql | 3 + schema/crdb/dbinit.sql | 7 - tools/opte_version | 2 +- workspace-hack/Cargo.toml | 2 - 8 files changed, 189 insertions(+), 158 deletions(-) create mode 100644 schema/crdb/14.0.0/up06.sql diff --git a/Cargo.lock b/Cargo.lock index 78f3fb9d017..d2ad4c471f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3099,7 +3099,7 @@ dependencies = [ [[package]] name = "illumos-sys-hdrs" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" +source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" [[package]] name = "illumos-utils" @@ -3501,7 +3501,7 @@ dependencies = [ [[package]] name = "kstat-macro" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" +source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" dependencies = [ "quote", "syn 2.0.32", @@ -4966,7 +4966,6 @@ dependencies = [ "clap_builder", "console", "const-oid", - "crc32fast", "crossbeam-epoch", "crossbeam-utils", "crossterm", @@ -5179,12 +5178,10 @@ dependencies = [ [[package]] name = "opte" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" +source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" dependencies = [ "cfg-if", - "crc32fast", "dyn-clone", - "heapless", "illumos-sys-hdrs", "kstat-macro", "opte-api", @@ -5192,13 +5189,12 @@ dependencies = [ "serde", "smoltcp 0.10.0", "version_check", - "zerocopy 0.7.26", ] [[package]] name = "opte-api" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" +source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" dependencies = [ "illumos-sys-hdrs", "ipnetwork", @@ -5210,7 +5206,7 @@ dependencies = [ [[package]] name = "opte-ioctl" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" +source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" dependencies = [ "libc", "libnet", @@ -5284,7 +5280,7 @@ dependencies = [ [[package]] name = "oxide-vpc" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=8e1b01baf5c161017a966f139fb774eb8f32d130#8e1b01baf5c161017a966f139fb774eb8f32d130" +source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" dependencies = [ "illumos-sys-hdrs", "opte", diff --git a/Cargo.toml b/Cargo.toml index 8bf90c9d733..e3a983f787c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -260,7 +260,7 @@ omicron-sled-agent = { path = "sled-agent" } omicron-test-utils = { path = "test-utils" } omicron-zone-package = "0.9.1" oxide-client = { path = "clients/oxide-client" } -oxide-vpc = { git = "https://github.com/oxidecomputer/opte", rev = "8e1b01baf5c161017a966f139fb774eb8f32d130", features = [ "api", "std" ] } +oxide-vpc = { git = "https://github.com/oxidecomputer/opte", rev = "d508dcd12cbfeb8b948c0cffe800899046322233", features = [ "api", "std" ] } once_cell = "1.18.0" openapi-lint = { git = "https://github.com/oxidecomputer/openapi-lint", branch = "main" } openapiv3 = "1.0" @@ -268,7 +268,7 @@ openapiv3 = "1.0" openssl = "0.10" openssl-sys = "0.9" openssl-probe = "0.1.5" -opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "8e1b01baf5c161017a966f139fb774eb8f32d130" } +opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "d508dcd12cbfeb8b948c0cffe800899046322233" } oso = "0.27" owo-colors = "3.5.0" oximeter = { path = "oximeter/oximeter" } diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index ccee30726e3..04549b58457 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -203,6 +203,7 @@ impl DataStore { conn: &async_bb8_diesel::Connection, data: IncompleteExternalIp, ) -> CreateResult { + use diesel::result::DatabaseErrorKind::UniqueViolation; // Name needs to be cloned out here (if present) to give users a // sensible error message on name collision. let name = data.name().clone(); @@ -222,7 +223,7 @@ impl DataStore { ) } } - DatabaseError(..) if name.is_some() => { + DatabaseError(UniqueViolation, ..) if name.is_some() => { public_error_from_diesel( e, ErrorHandler::Conflict( diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index 2456c798431..a7a5e8a9217 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -1675,6 +1675,7 @@ mod test { use crate::db::model::IpKind; use crate::db::schema::external_ip::dsl; use diesel::result::DatabaseErrorKind::CheckViolation; + use diesel::result::DatabaseErrorKind::UniqueViolation; use diesel::result::Error::DatabaseError; let logctx = dev::test_setup_log("test_external_ip_check_constraints"); @@ -1712,151 +1713,190 @@ mod test { // - name // - description // - parent (instance / service) UUID - let names = [ - None, - Some(db::model::Name(Name::try_from("foo".to_string()).unwrap())), - ]; + // - project UUID + let names = [None, Some("foo")]; let descriptions = [None, Some("foo".to_string())]; let parent_ids = [None, Some(Uuid::new_v4())]; + let project_ids = [None, Some(Uuid::new_v4())]; + + let mut seen_pairs = HashSet::new(); // For Floating IPs, both name and description must be non-NULL - for name in names.iter() { - for description in descriptions.iter() { - for parent_id in parent_ids.iter() { - for is_service in [false, true] { - let new_ip = ExternalIp { - id: Uuid::new_v4(), - name: name.clone(), - description: description.clone(), - ip: addresses.next().unwrap().into(), - is_service, - parent_id: *parent_id, - ..ip - }; - let res = diesel::insert_into(dsl::external_ip) - .values(new_ip) - .execute_async(&*conn) - .await; - if name.is_some() && description.is_some() { - // Name/description must be non-NULL, instance ID can be - // either - res.unwrap_or_else(|_| { - panic!( - "Failed to insert Floating IP with valid \ - name, description, and {} ID", - if is_service { - "Service" - } else { - "Instance" - } - ) - }); - } else { - // At least one is not valid, we expect a check violation - let err = res.expect_err( - "Expected a CHECK violation when inserting a \ - Floating IP record with NULL name and/or description", - ); - assert!( - matches!( - err, - DatabaseError( - CheckViolation, - _ - ) - ), - "Expected a CHECK violation when inserting a \ - Floating IP record with NULL name and/or description", - ); - } - } - } + // If they are instance FIPs, they *must* have a project id. + for ( + name, + description, + parent_id, + is_service, + project_id, + modify_name, + ) in itertools::iproduct!( + &names, + &descriptions, + &parent_ids, + [false, true], + &project_ids, + [false, true] + ) { + // Both choices of parent_id are valid, so we need a unique name for each. + let name_local = name.map(|v| { + let name = if modify_name { + v.to_string() + } else { + format!("{v}-with-parent") + }; + db::model::Name(Name::try_from(name).unwrap()) + }); + // We do name duplicate checking on the `Some` branch. + if parent_id.is_none() && modify_name { + continue; + } + + let new_ip = ExternalIp { + id: Uuid::new_v4(), + name: name_local.clone(), + description: description.clone(), + ip: addresses.next().unwrap().into(), + is_service, + parent_id: *parent_id, + project_id: *project_id, + ..ip + }; + + let key = (*project_id, name_local); + + let res = diesel::insert_into(dsl::external_ip) + .values(new_ip) + .execute_async(&*conn) + .await; + + let project_as_expected = (is_service && project_id.is_none()) + || (!is_service && project_id.is_some()); + + let valid_expression = + name.is_some() && description.is_some() && project_as_expected; + let name_exists = seen_pairs.contains(&key); + + if valid_expression && !name_exists { + // Name/description must be non-NULL, instance ID can be + // either + // Names must be unique at fleet level and at project level. + // Project must be NULL if service, non-NULL if instance. + res.unwrap_or_else(|e| { + panic!( + "Failed to insert Floating IP with valid \ + name, description, project ID, and {} ID:\ + {name:?} {description:?} {project_id:?} {:?}\n{e}", + if is_service { "Service" } else { "Instance" }, + &ip.parent_id + ) + }); + + seen_pairs.insert(key); + } else if !valid_expression { + // NOTE: CHECK violation will supersede UNIQUE violation below. + // At least one is not valid, we expect a check violation + let err = res.expect_err( + "Expected a CHECK violation when inserting a \ + Floating IP record with NULL name and/or description", + ); + assert!( + matches!(err, DatabaseError(CheckViolation, _)), + "Expected a CHECK violation when inserting a \ + Floating IP record with NULL name and/or description, \ + and incorrect project parent relation \ + \nGOT: {err:?}", + ); + } else { + // At least one is not valid, we expect a check violation + let err = res.expect_err( + "Expected a UNIQUE violation when inserting a \ + Floating IP record with existing (name, project_id)", + ); + assert!( + matches!(err, DatabaseError(UniqueViolation, _)), + "Expected a UNIQUE violation when inserting a \ + Floating IP record with existing (name, project_id) \ + \nGOT: {err:?}", + ); } } - // For other IP types, both name and description must be NULL - for kind in [IpKind::SNat, IpKind::Ephemeral].into_iter() { - for name in names.iter() { - for description in descriptions.iter() { - for parent_id in parent_ids.iter() { - for is_service in [false, true] { - let new_ip = ExternalIp { - id: Uuid::new_v4(), - name: name.clone(), - description: description.clone(), - kind, - ip: addresses.next().unwrap().into(), - is_service, - parent_id: *parent_id, - ..ip - }; - let res = diesel::insert_into(dsl::external_ip) - .values(new_ip.clone()) - .execute_async(&*conn) - .await; - let ip_type = - if is_service { "Service" } else { "Instance" }; - if name.is_none() - && description.is_none() - && parent_id.is_some() - { - // Name/description must be NULL, instance ID cannot - // be NULL. - - if kind == IpKind::Ephemeral && is_service { - // Ephemeral Service IPs aren't supported. - let err = res.unwrap_err(); - assert!( - matches!( - err, - DatabaseError( - CheckViolation, - _ - ) - ), - "Expected a CHECK violation when inserting an \ - Ephemeral Service IP", - ); - } else { - assert!( - res.is_ok(), - "Failed to insert {:?} IP with valid \ - name, description, and {} ID", - kind, - ip_type, - ); - } - } else { - // One is not valid, we expect a check violation - assert!( - res.is_err(), - "Expected a CHECK violation when inserting a \ - {:?} IP record with non-NULL name, description, \ - and/or {} ID", - kind, - ip_type, - ); - let err = res.unwrap_err(); - assert!( - matches!( - err, - DatabaseError( - CheckViolation, - _ - ) - ), - "Expected a CHECK violation when inserting a \ - {:?} IP record with non-NULL name, description, \ - and/or {} ID", - kind, - ip_type, - ); - } - } - } + // For other IP types: name, description and project must be NULL + for (kind, name, description, parent_id, is_service, project_id) in itertools::iproduct!( + [IpKind::SNat, IpKind::Ephemeral], + &names, + &descriptions, + &parent_ids, + [false, true], + &project_ids + ) { + let name_local = name.map(|v| { + db::model::Name(Name::try_from(v.to_string()).unwrap()) + }); + let new_ip = ExternalIp { + id: Uuid::new_v4(), + name: name_local, + description: description.clone(), + kind, + ip: addresses.next().unwrap().into(), + is_service, + parent_id: *parent_id, + project_id: *project_id, + ..ip + }; + let res = diesel::insert_into(dsl::external_ip) + .values(new_ip.clone()) + .execute_async(&*conn) + .await; + let ip_type = if is_service { "Service" } else { "Instance" }; + if name.is_none() + && description.is_none() + && parent_id.is_some() + && project_id.is_none() + { + // Name/description must be NULL, instance ID cannot + // be NULL. + + if kind == IpKind::Ephemeral && is_service { + // Ephemeral Service IPs aren't supported. + let err = res.unwrap_err(); + assert!( + matches!(err, DatabaseError(CheckViolation, _)), + "Expected a CHECK violation when inserting an \ + Ephemeral Service IP", + ); + } else { + assert!( + res.is_ok(), + "Failed to insert {:?} IP with valid \ + name, description, and {} ID", + kind, + ip_type, + ); } + } else { + // One is not valid, we expect a check violation + assert!( + res.is_err(), + "Expected a CHECK violation when inserting a \ + {:?} IP record with non-NULL name, description, \ + and/or {} ID", + kind, + ip_type, + ); + let err = res.unwrap_err(); + assert!( + matches!(err, DatabaseError(CheckViolation, _)), + "Expected a CHECK violation when inserting a \ + {:?} IP record with non-NULL name, description, \ + and/or {} ID", + kind, + ip_type, + ); } } + db.cleanup().await.unwrap(); logctx.cleanup_successful(); } diff --git a/schema/crdb/14.0.0/up06.sql b/schema/crdb/14.0.0/up06.sql new file mode 100644 index 00000000000..30c0b3773ad --- /dev/null +++ b/schema/crdb/14.0.0/up06.sql @@ -0,0 +1,3 @@ +ALTER TABLE omicron.public.external_ip ADD CONSTRAINT IF NOT EXISTS null_non_fip_parent_id CHECK ( + (kind != 'floating' AND parent_id is NOT NULL) OR (kind = 'floating') +); diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index b890dd19f8a..df815ffef47 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -1625,13 +1625,6 @@ CREATE TABLE IF NOT EXISTS omicron.public.external_ip ( ((kind != 'floating' OR is_service = TRUE) AND project_id IS NULL) ), - /* Ephemeral/SNAT IPs must have a parent pool and range, while this - * is optional for floating IPs. - */ - CONSTRAINT null_non_fip_pool_id CHECK ( - kind = 'floating' OR (ip_pool_id IS NOT NULL AND ip_pool_range_id IS NOT NULL) - ), - /* * Only nullable if this is a floating IP, which may exist not * attached to any instance or service yet. diff --git a/tools/opte_version b/tools/opte_version index 82d79dcf282..a8c459b83df 100644 --- a/tools/opte_version +++ b/tools/opte_version @@ -1 +1 @@ -0.27.214 +0.27.217 diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index 6ad4ed78bab..1a289bd0cbb 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -29,7 +29,6 @@ clap = { version = "4.4.3", features = ["derive", "env", "wrap_help"] } clap_builder = { version = "4.4.2", default-features = false, features = ["color", "env", "std", "suggestions", "usage", "wrap_help"] } console = { version = "0.15.7" } const-oid = { version = "0.9.5", default-features = false, features = ["db", "std"] } -crc32fast = { version = "1.3.2" } crossbeam-epoch = { version = "0.9.15" } crossbeam-utils = { version = "0.8.16" } crossterm = { version = "0.27.0", features = ["event-stream", "serde"] } @@ -125,7 +124,6 @@ clap = { version = "4.4.3", features = ["derive", "env", "wrap_help"] } clap_builder = { version = "4.4.2", default-features = false, features = ["color", "env", "std", "suggestions", "usage", "wrap_help"] } console = { version = "0.15.7" } const-oid = { version = "0.9.5", default-features = false, features = ["db", "std"] } -crc32fast = { version = "1.3.2" } crossbeam-epoch = { version = "0.9.15" } crossbeam-utils = { version = "0.8.16" } crossterm = { version = "0.27.0", features = ["event-stream", "serde"] } From 037f7b6d31cde6c2c7ca1658640ca1bdc543480a Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 27 Nov 2023 13:56:38 +0000 Subject: [PATCH 15/27] Fixup iam_roles_test with FloatingIp Resource --- .../src/authz/policy_test/resources.rs | 7 ++++ nexus/db-queries/tests/output/authz-roles.out | 42 +++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/nexus/db-queries/src/authz/policy_test/resources.rs b/nexus/db-queries/src/authz/policy_test/resources.rs index 3049f3b9bf0..8bdd97923b4 100644 --- a/nexus/db-queries/src/authz/policy_test/resources.rs +++ b/nexus/db-queries/src/authz/policy_test/resources.rs @@ -319,6 +319,13 @@ async fn make_project( Uuid::new_v4(), LookupType::ByName(image_name), )); + + let floating_ip_name = format!("{project_name}-fip1"); + builder.new_resource(authz::FloatingIp::new( + project.clone(), + Uuid::new_v4(), + LookupType::ByName(floating_ip_name), + )); } /// Returns the set of authz classes exempted from the coverage test diff --git a/nexus/db-queries/tests/output/authz-roles.out b/nexus/db-queries/tests/output/authz-roles.out index 963f00f7e85..54fb6481a9f 100644 --- a/nexus/db-queries/tests/output/authz-roles.out +++ b/nexus/db-queries/tests/output/authz-roles.out @@ -376,6 +376,20 @@ resource: ProjectImage "silo1-proj1-image1" silo1-proj1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! +resource: FloatingIp "silo1-proj1-fip1" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-proj1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-proj1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + resource: Project "silo1-proj2" USER Q R LC RP M MP CC D @@ -488,6 +502,20 @@ resource: ProjectImage "silo1-proj2-image1" silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! +resource: FloatingIp "silo1-proj2-fip1" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-collaborator ✘ ✔ ✔ ✔ ✔ ✔ ✔ ✔ + silo1-viewer ✘ ✔ ✔ ✔ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + resource: Silo "silo2" USER Q R LC RP M MP CC D @@ -768,6 +796,20 @@ resource: ProjectImage "silo2-proj1-image1" silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ unauthenticated ! ! ! ! ! ! ! ! +resource: FloatingIp "silo2-proj1-fip1" + + USER Q R LC RP M MP CC D + fleet-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + fleet-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-admin ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-collaborator ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + silo1-proj1-viewer ✘ ✘ ✘ ✘ ✘ ✘ ✘ ✘ + unauthenticated ! ! ! ! ! ! ! ! + resource: Rack id "c037e882-8b6d-c8b5-bef4-97e848eb0a50" USER Q R LC RP M MP CC D From 9edb4ea4c108d39434204cb0e86bd75bdc2f8c18 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 27 Nov 2023 20:47:13 +0000 Subject: [PATCH 16/27] Fix up authz integration tests, add ephemeral limit --- .../omicron-dev/tests/test_omicron_dev.rs | 2 +- nexus/src/app/instance.rs | 14 +++++++ nexus/src/app/mod.rs | 1 + nexus/tests/integration_tests/endpoints.rs | 40 +++++++++++++++++++ nexus/tests/integration_tests/unauthorized.rs | 6 +++ nexus/types/src/external_api/params.rs | 2 - 6 files changed, 62 insertions(+), 3 deletions(-) diff --git a/dev-tools/omicron-dev/tests/test_omicron_dev.rs b/dev-tools/omicron-dev/tests/test_omicron_dev.rs index f1e81772433..88d92a7f69c 100644 --- a/dev-tools/omicron-dev/tests/test_omicron_dev.rs +++ b/dev-tools/omicron-dev/tests/test_omicron_dev.rs @@ -27,7 +27,7 @@ use subprocess::Redirection; const CMD_OMICRON_DEV: &str = env!("CARGO_BIN_EXE_omicron-dev"); /// timeout used for various things that should be pretty quick -const TIMEOUT: Duration = Duration::from_secs(15); +const TIMEOUT: Duration = Duration::from_secs(60); fn path_to_omicron_dev() -> PathBuf { path_to_executable(CMD_OMICRON_DEV) diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 21461b87800..76e9a0b4ffe 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -5,6 +5,7 @@ //! Virtual Machine Instances use super::MAX_DISKS_PER_INSTANCE; +use super::MAX_EPHEMERAL_IPS_PER_INSTANCE; use super::MAX_EXTERNAL_IPS_PER_INSTANCE; use super::MAX_MEMORY_BYTES_PER_INSTANCE; use super::MAX_NICS_PER_INSTANCE; @@ -52,6 +53,7 @@ use sled_agent_client::types::InstanceProperties; use sled_agent_client::types::InstancePutMigrationIdsBody; use sled_agent_client::types::InstancePutStateBody; use sled_agent_client::types::SourceNatConfig; +use std::matches; use std::net::SocketAddr; use std::sync::Arc; use tokio::io::{AsyncRead, AsyncWrite}; @@ -168,6 +170,18 @@ impl super::Nexus { MAX_EXTERNAL_IPS_PER_INSTANCE, ))); } + if params + .external_ips + .iter() + .filter(|v| matches!(v, params::ExternalIpCreate::Ephemeral { .. })) + .count() + > MAX_EPHEMERAL_IPS_PER_INSTANCE + { + return Err(Error::invalid_request(&format!( + "An instance may not have more than {} ephemeral IP address", + MAX_EPHEMERAL_IPS_PER_INSTANCE, + ))); + } if let params::InstanceNetworkInterfaceAttachment::Create(ref ifaces) = params.network_interfaces { diff --git a/nexus/src/app/mod.rs b/nexus/src/app/mod.rs index b04344b2a44..a3b2e963604 100644 --- a/nexus/src/app/mod.rs +++ b/nexus/src/app/mod.rs @@ -82,6 +82,7 @@ pub(crate) const MAX_NICS_PER_INSTANCE: usize = 8; // XXX: Might want to recast as max *floating* IPs, we have at most one // ephemeral (so bounded in saga by design). pub(crate) const MAX_EXTERNAL_IPS_PER_INSTANCE: usize = 32; +pub(crate) const MAX_EPHEMERAL_IPS_PER_INSTANCE: usize = 1; pub const MAX_VCPU_PER_INSTANCE: u16 = 64; diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 64790c49c20..a421f7a701a 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -115,6 +115,7 @@ lazy_static! { pub static ref DEMO_PROJECT_URL_INSTANCES: String = format!("/v1/instances?project={}", *DEMO_PROJECT_NAME); pub static ref DEMO_PROJECT_URL_SNAPSHOTS: String = format!("/v1/snapshots?project={}", *DEMO_PROJECT_NAME); pub static ref DEMO_PROJECT_URL_VPCS: String = format!("/v1/vpcs?project={}", *DEMO_PROJECT_NAME); + pub static ref DEMO_PROJECT_URL_FIPS: String = format!("/v1/floating-ips?project={}", *DEMO_PROJECT_NAME); pub static ref DEMO_PROJECT_CREATE: params::ProjectCreate = params::ProjectCreate { identity: IdentityMetadataCreateParams { @@ -554,6 +555,22 @@ lazy_static! { }; } +lazy_static! { + // Project Floating IPs + pub static ref DEMO_FLOAT_IP_NAME: Name = "float-ip".parse().unwrap(); + pub static ref DEMO_FLOAT_IP_URL: String = + format!("/v1/floating-ips/{}?project={}", *DEMO_FLOAT_IP_NAME, *DEMO_PROJECT_NAME); + pub static ref DEMO_FLOAT_IP_CREATE: params::FloatingIpCreate = + params::FloatingIpCreate { + identity: IdentityMetadataCreateParams { + name: DEMO_FLOAT_IP_NAME.clone(), + description: String::from("a new IP pool"), + }, + address: Some(std::net::Ipv4Addr::new(10, 0, 0, 141).into()), + pool: None, + }; +} + lazy_static! { // Identity providers pub static ref IDENTITY_PROVIDERS_URL: String = format!("/v1/system/identity-providers?silo=demo-silo"); @@ -1961,6 +1978,29 @@ lazy_static! { allowed_methods: vec![ AllowedMethod::GetNonexistent, ], + }, + + // Floating IPs + VerifyEndpoint { + url: &DEMO_PROJECT_URL_FIPS, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Post( + serde_json::to_value(&*DEMO_FLOAT_IP_CREATE).unwrap(), + ), + AllowedMethod::Get, + ], + }, + + VerifyEndpoint { + url: &DEMO_FLOAT_IP_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get, + AllowedMethod::Delete, + ], } ]; } diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 9936af20bf0..1cb2eaca3a4 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -278,6 +278,12 @@ lazy_static! { body: serde_json::to_value(&*DEMO_IMAGE_CREATE).unwrap(), id_routes: vec!["/v1/images/{id}"], }, + // Create a Floating IP in the project + SetupReq::Post { + url: &DEMO_PROJECT_URL_FIPS, + body: serde_json::to_value(&*DEMO_FLOAT_IP_CREATE).unwrap(), + id_routes: vec!["/v1/floating-ips/{id}"], + }, // Create a SAML identity provider SetupReq::Post { url: &SAML_IDENTITY_PROVIDERS_URL, diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 12dd01299b3..13433d0001f 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -770,13 +770,11 @@ pub struct FloatingIpCreate { /// An IP address to reserve for use as a floating IP. This field is /// optional if a pool is provided, in which case an address will /// be automatically chosen from there. - // TODO: draw from pool if needed. pub address: Option, /// The parent IP pool that a floating IP is pulled from. If combined /// with an explicit address, then that address must be available in /// the pool. - // TODO: support tie-in to pools. pub pool: Option, } From 4b6029edb526718198f6b87286f6f82acf829de6 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 27 Nov 2023 22:16:38 +0000 Subject: [PATCH 17/27] Move new changes to Schema v15.0 --- nexus/db-model/src/schema.rs | 2 +- schema/crdb/{14.0.0 => 15.0.0}/up01.sql | 0 schema/crdb/{14.0.0 => 15.0.0}/up02.sql | 0 schema/crdb/{14.0.0 => 15.0.0}/up03.sql | 0 schema/crdb/{14.0.0 => 15.0.0}/up04.sql | 0 schema/crdb/{14.0.0 => 15.0.0}/up05.sql | 0 schema/crdb/{14.0.0 => 15.0.0}/up06.sql | 0 schema/crdb/dbinit.sql | 2 +- 8 files changed, 2 insertions(+), 2 deletions(-) rename schema/crdb/{14.0.0 => 15.0.0}/up01.sql (100%) rename schema/crdb/{14.0.0 => 15.0.0}/up02.sql (100%) rename schema/crdb/{14.0.0 => 15.0.0}/up03.sql (100%) rename schema/crdb/{14.0.0 => 15.0.0}/up04.sql (100%) rename schema/crdb/{14.0.0 => 15.0.0}/up05.sql (100%) rename schema/crdb/{14.0.0 => 15.0.0}/up06.sql (100%) diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index fb60a7aca5a..3af5e5755cd 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -1320,7 +1320,7 @@ table! { /// /// This should be updated whenever the schema is changed. For more details, /// refer to: schema/crdb/README.adoc -pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(14, 0, 0); +pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(15, 0, 0); allow_tables_to_appear_in_same_query!( system_update, diff --git a/schema/crdb/14.0.0/up01.sql b/schema/crdb/15.0.0/up01.sql similarity index 100% rename from schema/crdb/14.0.0/up01.sql rename to schema/crdb/15.0.0/up01.sql diff --git a/schema/crdb/14.0.0/up02.sql b/schema/crdb/15.0.0/up02.sql similarity index 100% rename from schema/crdb/14.0.0/up02.sql rename to schema/crdb/15.0.0/up02.sql diff --git a/schema/crdb/14.0.0/up03.sql b/schema/crdb/15.0.0/up03.sql similarity index 100% rename from schema/crdb/14.0.0/up03.sql rename to schema/crdb/15.0.0/up03.sql diff --git a/schema/crdb/14.0.0/up04.sql b/schema/crdb/15.0.0/up04.sql similarity index 100% rename from schema/crdb/14.0.0/up04.sql rename to schema/crdb/15.0.0/up04.sql diff --git a/schema/crdb/14.0.0/up05.sql b/schema/crdb/15.0.0/up05.sql similarity index 100% rename from schema/crdb/14.0.0/up05.sql rename to schema/crdb/15.0.0/up05.sql diff --git a/schema/crdb/14.0.0/up06.sql b/schema/crdb/15.0.0/up06.sql similarity index 100% rename from schema/crdb/14.0.0/up06.sql rename to schema/crdb/15.0.0/up06.sql diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 4c8b5f3b467..799a020a85c 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -3045,7 +3045,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - ( TRUE, NOW(), NOW(), '14.0.0', NULL) + ( TRUE, NOW(), NOW(), '15.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; From e6cc0fb1a823b1dd6cd7dd926802cfc12400f13d Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 28 Nov 2023 14:32:20 +0000 Subject: [PATCH 18/27] Add unit tests for Floating API endpoint behaviour --- nexus/test-utils/src/resource_helpers.rs | 25 + nexus/tests/integration_tests/external_ips.rs | 429 ++++++++++++++++++ nexus/tests/integration_tests/instances.rs | 55 +++ nexus/tests/integration_tests/mod.rs | 1 + 4 files changed, 510 insertions(+) create mode 100644 nexus/tests/integration_tests/external_ips.rs diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 2368c3f5689..1848989bf98 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -21,6 +21,7 @@ use nexus_types::external_api::shared::IdentityType; use nexus_types::external_api::shared::IpRange; use nexus_types::external_api::views; use nexus_types::external_api::views::Certificate; +use nexus_types::external_api::views::FloatingIp; use nexus_types::external_api::views::IpPool; use nexus_types::external_api::views::IpPoolRange; use nexus_types::external_api::views::User; @@ -32,7 +33,9 @@ use omicron_common::api::external::Disk; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::Instance; use omicron_common::api::external::InstanceCpuCount; +use omicron_common::api::external::NameOrId; use omicron_sled_agent::sim::SledAgent; +use std::net::IpAddr; use std::sync::Arc; use uuid::Uuid; @@ -149,6 +152,28 @@ pub async fn create_ip_pool( (pool, range) } +pub async fn create_floating_ip( + client: &ClientTestContext, + fip_name: &str, + project: &str, + address: Option, + parent_pool_name: Option<&str>, +) -> FloatingIp { + object_create( + client, + &format!("/v1/floating-ips?project={project}"), + ¶ms::FloatingIpCreate { + identity: IdentityMetadataCreateParams { + name: fip_name.parse().unwrap(), + description: String::from("a floating ip"), + }, + address, + pool: parent_pool_name.map(|v| NameOrId::Name(v.parse().unwrap())), + }, + ) + .await +} + pub async fn create_certificate( client: &ClientTestContext, cert_name: &str, diff --git a/nexus/tests/integration_tests/external_ips.rs b/nexus/tests/integration_tests/external_ips.rs new file mode 100644 index 00000000000..f32f973fdeb --- /dev/null +++ b/nexus/tests/integration_tests/external_ips.rs @@ -0,0 +1,429 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! Tests Floating IP support in the API + +use std::net::IpAddr; +use std::net::Ipv4Addr; + +use crate::integration_tests::instances::instance_simulate; +use dropshot::test_util::ClientTestContext; +use dropshot::HttpErrorResponseBody; +use http::Method; +use http::StatusCode; +use nexus_test_utils::http_testing::AuthnMode; +use nexus_test_utils::http_testing::NexusRequest; +use nexus_test_utils::http_testing::RequestBuilder; +use nexus_test_utils::resource_helpers::create_floating_ip; +use nexus_test_utils::resource_helpers::create_instance_with; +use nexus_test_utils::resource_helpers::create_ip_pool; +use nexus_test_utils::resource_helpers::create_project; +use nexus_test_utils::resource_helpers::populate_ip_pool; +use nexus_test_utils_macros::nexus_test; +use nexus_types::external_api::params; +use nexus_types::external_api::views::FloatingIp; +use omicron_common::address::IpRange; +use omicron_common::address::Ipv4Range; +use omicron_common::api::external::IdentityMetadataCreateParams; +use omicron_common::api::external::Instance; +use uuid::Uuid; + +type ControlPlaneTestContext = + nexus_test_utils::ControlPlaneTestContext; + +const PROJECT_NAME: &str = "rootbeer-float"; + +const FIP_NAMES: &[&str] = &["vanilla", "chocolate", "strawberry", "pistachio"]; + +fn get_floating_ips_url(project_name: &str) -> String { + format!("/v1/floating-ips?project={project_name}") +} + +fn get_floating_ip_by_name_url(fip_name: &str, project_name: &str) -> String { + format!("/v1/floating-ips/{fip_name}?project={project_name}") +} + +fn get_floating_ip_by_id_url(fip_id: &Uuid) -> String { + format!("/v1/floating-ips/{fip_id}") +} + +#[nexus_test] +async fn test_floating_ip_access(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + + populate_ip_pool(&client, "default", None).await; + let project = create_project(client, PROJECT_NAME).await; + + // Create a floating IP from the default pool. + let fip_name = FIP_NAMES[0]; + let fip = create_floating_ip( + client, + fip_name, + &project.identity.id.to_string(), + None, + None, + ) + .await; + + // Fetch floating IP by ID + let fetched_fip = + floating_ip_get(&client, &get_floating_ip_by_id_url(&fip.identity.id)) + .await; + assert_eq!(fetched_fip.identity.id, fip.identity.id); + + // Fetch floating IP by name and project_id + let fetched_fip = floating_ip_get( + &client, + &get_floating_ip_by_name_url( + fip.identity.name.as_str(), + &project.identity.id.to_string(), + ), + ) + .await; + assert_eq!(fetched_fip.identity.id, fip.identity.id); + + // Fetch floating IP by name and project_name + let fetched_fip = floating_ip_get( + &client, + &get_floating_ip_by_name_url( + fip.identity.name.as_str(), + project.identity.name.as_str(), + ), + ) + .await; + assert_eq!(fetched_fip.identity.id, fip.identity.id); +} + +#[nexus_test] +async fn test_floating_ip_create(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + + populate_ip_pool(&client, "default", None).await; + let other_pool_range = IpRange::V4( + Ipv4Range::new(Ipv4Addr::new(10, 1, 0, 1), Ipv4Addr::new(10, 1, 0, 5)) + .unwrap(), + ); + create_ip_pool(&client, "other-pool", Some(other_pool_range)).await; + + let project = create_project(client, PROJECT_NAME).await; + + // Create with no chosen IP and fallback to default pool. + let fip_name = FIP_NAMES[0]; + let fip = create_floating_ip( + client, + fip_name, + project.identity.name.as_str(), + None, + None, + ) + .await; + assert_eq!(fip.identity.name.as_str(), fip_name); + assert_eq!(fip.project_id, project.identity.id); + assert_eq!(fip.instance_id, None); + assert_eq!(fip.ip, IpAddr::from(Ipv4Addr::new(10, 0, 0, 0))); + + // Create with chosen IP and fallback to default pool. + let fip_name = FIP_NAMES[1]; + let ip_addr = "10.0.12.34".parse().unwrap(); + let fip = create_floating_ip( + client, + fip_name, + project.identity.name.as_str(), + Some(ip_addr), + None, + ) + .await; + assert_eq!(fip.identity.name.as_str(), fip_name); + assert_eq!(fip.project_id, project.identity.id); + assert_eq!(fip.instance_id, None); + assert_eq!(fip.ip, ip_addr); + + // Create with no chosen IP from named pool. + let fip_name = FIP_NAMES[2]; + let fip = create_floating_ip( + client, + fip_name, + project.identity.name.as_str(), + None, + Some("other-pool"), + ) + .await; + assert_eq!(fip.identity.name.as_str(), fip_name); + assert_eq!(fip.project_id, project.identity.id); + assert_eq!(fip.instance_id, None); + assert_eq!(fip.ip, IpAddr::from(Ipv4Addr::new(10, 1, 0, 1))); + + // Create with chosen IP from named pool. + let fip_name = FIP_NAMES[3]; + let ip_addr = "10.1.0.5".parse().unwrap(); + let fip = create_floating_ip( + client, + fip_name, + project.identity.name.as_str(), + Some(ip_addr), + Some("other-pool"), + ) + .await; + assert_eq!(fip.identity.name.as_str(), fip_name); + assert_eq!(fip.project_id, project.identity.id); + assert_eq!(fip.instance_id, None); + assert_eq!(fip.ip, ip_addr); +} + +#[nexus_test] +async fn test_floating_ip_create_ip_in_use( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + populate_ip_pool(&client, "default", None).await; + + let project = create_project(client, PROJECT_NAME).await; + let contested_ip = "10.0.0.0".parse().unwrap(); + + // First create will succeed. + create_floating_ip( + client, + FIP_NAMES[0], + project.identity.name.as_str(), + Some(contested_ip), + None, + ) + .await; + + // Second will fail as the requested IP is in use in the selected + // (default) pool. + let error: HttpErrorResponseBody = NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &get_floating_ips_url(PROJECT_NAME), + ) + .body(Some(¶ms::FloatingIpCreate { + identity: IdentityMetadataCreateParams { + name: FIP_NAMES[1].parse().unwrap(), + description: "another fip".into(), + }, + address: Some(contested_ip), + pool: None, + })) + .expect_status(Some(StatusCode::BAD_REQUEST)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + assert_eq!(error.message, "Requested external IP address not available"); +} + +#[nexus_test] +async fn test_floating_ip_create_name_in_use( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + populate_ip_pool(&client, "default", None).await; + + let project = create_project(client, PROJECT_NAME).await; + let contested_name = FIP_NAMES[0]; + + // First create will succeed. + create_floating_ip( + client, + contested_name, + project.identity.name.as_str(), + None, + None, + ) + .await; + + // Second will fail as the requested name is in use within this + // project. + let error: HttpErrorResponseBody = NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &get_floating_ips_url(PROJECT_NAME), + ) + .body(Some(¶ms::FloatingIpCreate { + identity: IdentityMetadataCreateParams { + name: contested_name.parse().unwrap(), + description: "another fip".into(), + }, + address: None, + pool: None, + })) + .expect_status(Some(StatusCode::BAD_REQUEST)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + assert_eq!( + error.message, + format!("already exists: floating-ip \"{contested_name}\""), + ); +} + +#[nexus_test] +async fn test_floating_ip_delete(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + + populate_ip_pool(&client, "default", None).await; + let project = create_project(client, PROJECT_NAME).await; + + let fip = create_floating_ip( + client, + FIP_NAMES[0], + project.identity.name.as_str(), + None, + None, + ) + .await; + + // Delete the floating IP. + NexusRequest::object_delete( + client, + &get_floating_ip_by_id_url(&fip.identity.id), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); +} + +#[nexus_test] +async fn test_floating_ip_attachment(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let apictx = &cptestctx.server.apictx(); + let nexus = &apictx.nexus; + + populate_ip_pool(&client, "default", None).await; + let project = create_project(client, PROJECT_NAME).await; + + let fip = create_floating_ip( + client, + FIP_NAMES[0], + project.identity.name.as_str(), + None, + None, + ) + .await; + + // Bind the floating IP to an instance at create time. + let instance_name = "anonymous-diner"; + let instance = create_instance_with( + &client, + PROJECT_NAME, + instance_name, + ¶ms::InstanceNetworkInterfaceAttachment::Default, + vec![], + vec![params::ExternalIpCreate::Floating { + floating_ip_name: FIP_NAMES[0].parse().unwrap(), + }], + ) + .await; + + // Reacquire FIP: parent ID must have updated to match instance. + let fetched_fip = + floating_ip_get(&client, &get_floating_ip_by_id_url(&fip.identity.id)) + .await; + assert_eq!(fetched_fip.instance_id, Some(instance.identity.id)); + + // Try to delete the floating IP, which should fail. + let error: HttpErrorResponseBody = NexusRequest::new( + RequestBuilder::new( + client, + Method::DELETE, + &get_floating_ip_by_id_url(&fip.identity.id), + ) + .expect_status(Some(StatusCode::BAD_REQUEST)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + assert_eq!( + error.message, + format!("Floating IP cannot be deleted while attached to an instance"), + ); + + // Stop and delete the instance. + instance_simulate(nexus, &instance.identity.id).await; + instance_simulate(nexus, &instance.identity.id).await; + + let _: Instance = NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &format!("/v1/instances/{}/stop", instance.identity.id), + ) + .body(None as Option<&serde_json::Value>) + .expect_status(Some(StatusCode::ACCEPTED)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + + instance_simulate(nexus, &instance.identity.id).await; + + NexusRequest::object_delete( + &client, + &format!("/v1/instances/{instance_name}?project={PROJECT_NAME}"), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); + + // Reacquire FIP again: parent ID must now be unset. + let fetched_fip = + floating_ip_get(&client, &get_floating_ip_by_id_url(&fip.identity.id)) + .await; + assert_eq!(fetched_fip.instance_id, None); + + // Delete the floating IP. + NexusRequest::object_delete( + client, + &get_floating_ip_by_id_url(&fip.identity.id), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); +} + +async fn floating_ip_get( + client: &ClientTestContext, + fip_url: &str, +) -> FloatingIp { + floating_ip_get_as(client, fip_url, AuthnMode::PrivilegedUser).await +} + +async fn floating_ip_get_as( + client: &ClientTestContext, + fip_url: &str, + authn_as: AuthnMode, +) -> FloatingIp { + NexusRequest::object_get(client, fip_url) + .authn_as(authn_as) + .execute() + .await + .unwrap_or_else(|e| { + panic!("failed to make \"get\" request to {fip_url}: {e}") + }) + .parsed_body() + .unwrap_or_else(|e| { + panic!("failed to make \"get\" request to {fip_url}: {e}") + }) +} diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index ea633be9dcc..07021e39436 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -3645,6 +3645,61 @@ async fn test_instance_ephemeral_ip_from_correct_pool( ); } +#[nexus_test] +async fn test_instance_allow_only_one_ephemeral_ip( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + let _ = create_project(&client, PROJECT_NAME).await; + + // Create one IP pool with space for two ephemerals. + let default_pool_range = IpRange::V4( + Ipv4Range::new( + std::net::Ipv4Addr::new(10, 0, 0, 1), + std::net::Ipv4Addr::new(10, 0, 0, 2), + ) + .unwrap(), + ); + populate_ip_pool(&client, "default", Some(default_pool_range)).await; + + let ephemeral_create = params::ExternalIpCreate::Ephemeral { + pool_name: Some("default".parse().unwrap()), + }; + let error: HttpErrorResponseBody = NexusRequest::new( + RequestBuilder::new(client, Method::POST, &get_instances_url()) + .body(Some(¶ms::InstanceCreate { + identity: IdentityMetadataCreateParams { + name: "default-pool-inst".parse().unwrap(), + description: "instance default-pool-inst".into(), + }, + ncpus: InstanceCpuCount(4), + memory: ByteCount::from_gibibytes_u32(1), + hostname: String::from("the_host"), + user_data: + b"#cloud-config\nsystem_info:\n default_user:\n name: oxide" + .to_vec(), + network_interfaces: params::InstanceNetworkInterfaceAttachment::Default, + external_ips: vec![ + ephemeral_create.clone(), ephemeral_create + ], + disks: vec![], + start: true, + })) + .expect_status(Some(StatusCode::BAD_REQUEST)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + assert_eq!( + error.message, + "An instance may not have more than 1 ephemeral IP address" + ); +} + async fn create_instance_with_pool( client: &ClientTestContext, instance_name: &str, diff --git a/nexus/tests/integration_tests/mod.rs b/nexus/tests/integration_tests/mod.rs index 87c5c74f0fa..df45f1e11ae 100644 --- a/nexus/tests/integration_tests/mod.rs +++ b/nexus/tests/integration_tests/mod.rs @@ -12,6 +12,7 @@ mod commands; mod console_api; mod device_auth; mod disks; +mod external_ips; mod images; mod initialization; mod instances; From e037e3e952809266fea50d538be20563daeaa215 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 28 Nov 2023 17:04:41 +0000 Subject: [PATCH 19/27] Add multi-external-IP test for instances --- nexus/tests/integration_tests/external_ips.rs | 11 +- nexus/tests/integration_tests/instances.rs | 103 +++++++++++++++++- 2 files changed, 105 insertions(+), 9 deletions(-) diff --git a/nexus/tests/integration_tests/external_ips.rs b/nexus/tests/integration_tests/external_ips.rs index f32f973fdeb..f3161dea729 100644 --- a/nexus/tests/integration_tests/external_ips.rs +++ b/nexus/tests/integration_tests/external_ips.rs @@ -36,15 +36,18 @@ const PROJECT_NAME: &str = "rootbeer-float"; const FIP_NAMES: &[&str] = &["vanilla", "chocolate", "strawberry", "pistachio"]; -fn get_floating_ips_url(project_name: &str) -> String { +pub fn get_floating_ips_url(project_name: &str) -> String { format!("/v1/floating-ips?project={project_name}") } -fn get_floating_ip_by_name_url(fip_name: &str, project_name: &str) -> String { +pub fn get_floating_ip_by_name_url( + fip_name: &str, + project_name: &str, +) -> String { format!("/v1/floating-ips/{fip_name}?project={project_name}") } -fn get_floating_ip_by_id_url(fip_id: &Uuid) -> String { +pub fn get_floating_ip_by_id_url(fip_id: &Uuid) -> String { format!("/v1/floating-ips/{fip_id}") } @@ -403,7 +406,7 @@ async fn test_floating_ip_attachment(cptestctx: &ControlPlaneTestContext) { .unwrap(); } -async fn floating_ip_get( +pub async fn floating_ip_get( client: &ClientTestContext, fip_url: &str, ) -> FloatingIp { diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index 07021e39436..dffff1a4728 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -4,11 +4,15 @@ //! Tests basic instance support in the API +use crate::integration_tests::external_ips::floating_ip_get; +use crate::integration_tests::external_ips::get_floating_ip_by_id_url; + use super::metrics::{get_latest_silo_metric, get_latest_system_metric}; use camino::Utf8Path; use http::method::Method; use http::StatusCode; +use itertools::Itertools; use nexus_db_queries::context::OpContext; use nexus_db_queries::db::fixed_data::silo::DEFAULT_SILO; use nexus_db_queries::db::fixed_data::silo::SILO_ID; @@ -18,6 +22,7 @@ use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; use nexus_test_utils::resource_helpers::create_disk; +use nexus_test_utils::resource_helpers::create_floating_ip; use nexus_test_utils::resource_helpers::create_ip_pool; use nexus_test_utils::resource_helpers::create_local_user; use nexus_test_utils::resource_helpers::create_silo; @@ -54,6 +59,7 @@ use omicron_nexus::TestInterfaces as _; use omicron_sled_agent::sim::SledAgent; use sled_agent_client::TestInterfaces as _; use std::convert::TryFrom; +use std::net::Ipv4Addr; use std::sync::Arc; use uuid::Uuid; @@ -3645,6 +3651,84 @@ async fn test_instance_ephemeral_ip_from_correct_pool( ); } +#[nexus_test] +async fn test_instance_attach_several_external_ips( + cptestctx: &ControlPlaneTestContext, +) { + let client = &cptestctx.external_client; + + let _ = create_project(&client, PROJECT_NAME).await; + + // Create a single (large) IP pool + let default_pool_range = IpRange::V4( + Ipv4Range::new( + std::net::Ipv4Addr::new(10, 0, 0, 1), + std::net::Ipv4Addr::new(10, 0, 0, 10), + ) + .unwrap(), + ); + populate_ip_pool(&client, "default", Some(default_pool_range)).await; + + // Create several floating IPs for the instance, totalling 8 IPs. + let mut external_ip_create = + vec![params::ExternalIpCreate::Ephemeral { pool_name: None }]; + let mut fips = vec![]; + for i in 1..8 { + let name = format!("fip-{i}"); + fips.push( + create_floating_ip(&client, &name, PROJECT_NAME, None, None).await, + ); + external_ip_create.push(params::ExternalIpCreate::Floating { + floating_ip_name: name.parse().unwrap(), + }); + } + + // Create an instance with pool name blank, expect IP from default pool + let instance_name = "many-fips"; + let instance = create_instance_with( + &client, + PROJECT_NAME, + instance_name, + ¶ms::InstanceNetworkInterfaceAttachment::Default, + vec![], + external_ip_create, + ) + .await; + + // Verify that all external IPs are visible on the instance and have + // been allocated in order. + let external_ips = + fetch_instance_external_ips(&client, instance_name).await; + assert_eq!(external_ips.len(), 8); + eprintln!("{external_ips:?}"); + for (i, eip) in external_ips + .iter() + .sorted_unstable_by(|a, b| a.ip.cmp(&b.ip)) + .enumerate() + { + let last_octet = i + if i != external_ips.len() - 1 { + assert_eq!(eip.kind, IpKind::Floating); + 1 + } else { + // SNAT will occupy 1.0.0.8 here, since it it alloc'd before + // the ephemeral. + assert_eq!(eip.kind, IpKind::Ephemeral); + 2 + }; + assert_eq!(eip.ip, Ipv4Addr::new(10, 0, 0, last_octet as u8)); + } + + // Verify that all floating IPs are bound to their parent instance. + for fip in fips { + let fetched_fip = floating_ip_get( + &client, + &get_floating_ip_by_id_url(&fip.identity.id), + ) + .await; + assert_eq!(fetched_fip.instance_id, Some(instance.identity.id)); + } +} + #[nexus_test] async fn test_instance_allow_only_one_ephemeral_ip( cptestctx: &ControlPlaneTestContext, @@ -3718,10 +3802,10 @@ async fn create_instance_with_pool( .await } -async fn fetch_instance_ephemeral_ip( +async fn fetch_instance_external_ips( client: &ClientTestContext, instance_name: &str, -) -> views::ExternalIp { +) -> Vec { let ips_url = format!( "/v1/instances/{}/external-ips?project={}", instance_name, PROJECT_NAME @@ -3733,9 +3817,18 @@ async fn fetch_instance_ephemeral_ip( .expect("Failed to fetch external IPs") .parsed_body::>() .expect("Failed to parse external IPs"); - assert_eq!(ips.items.len(), 1); - assert_eq!(ips.items[0].kind, IpKind::Ephemeral); - ips.items[0].clone() + ips.items +} + +async fn fetch_instance_ephemeral_ip( + client: &ClientTestContext, + instance_name: &str, +) -> views::ExternalIp { + fetch_instance_external_ips(client, instance_name) + .await + .into_iter() + .find(|v| v.kind == IpKind::Ephemeral) + .unwrap() } #[nexus_test] From 5a5a02aaa65861d5a1f59caeb8d9f2546ab2dd4f Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 28 Nov 2023 19:46:09 +0000 Subject: [PATCH 20/27] Re-read complete changelog, tidying up. --- nexus/db-model/src/external_ip.rs | 6 ++++-- nexus/db-queries/src/db/datastore/external_ip.rs | 10 ++++------ nexus/db-queries/src/db/datastore/mod.rs | 16 ++++++++-------- nexus/src/app/instance.rs | 5 +++-- nexus/src/app/sagas/instance_create.rs | 3 ++- nexus/tests/integration_tests/instances.rs | 5 ++--- openapi/nexus.json | 4 ++-- 7 files changed, 25 insertions(+), 24 deletions(-) diff --git a/nexus/db-model/src/external_ip.rs b/nexus/db-model/src/external_ip.rs index bbaa8045dad..1a755f0396b 100644 --- a/nexus/db-model/src/external_ip.rs +++ b/nexus/db-model/src/external_ip.rs @@ -6,7 +6,8 @@ //! services. use crate::impl_enum_type; -use crate::schema::{external_ip, floating_ip}; +use crate::schema::external_ip; +use crate::schema::floating_ip; use crate::Name; use crate::SqlU16; use chrono::DateTime; @@ -20,7 +21,8 @@ use nexus_types::external_api::views; use omicron_common::address::NUM_SOURCE_NAT_PORTS; use omicron_common::api::external::Error; use omicron_common::api::external::IdentityMetadata; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; +use serde::Serialize; use std::convert::TryFrom; use std::net::IpAddr; use uuid::Uuid; diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index 04549b58457..41e9f657d34 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -145,8 +145,6 @@ impl DataStore { ) -> CreateResult { let ip_id = Uuid::new_v4(); - // XXX: mux here to scan *all* project pools in - // current silo for convenience? let pool_id = match params.pool { Some(NameOrId::Name(name)) => { LookupPath::new(opctx, self) @@ -343,8 +341,8 @@ impl DataStore { /// Detach an individual Floating IP address from its parent instance. /// - /// As in `deallocate_external_ip_by_instance_id`, This method returns the - /// number of records deleted, rather than the usual `DeleteResult`. + /// As in `deallocate_external_ip_by_instance_id`, this method returns the + /// number of records altered, rather than an `UpdateResult`. pub async fn detach_floating_ips_by_instance_id( &self, opctx: &OpContext, @@ -386,9 +384,9 @@ impl DataStore { authz_project: &authz::Project, pagparams: &PaginatedBy<'_>, ) -> ListResultVec { - opctx.authorize(authz::Action::ListChildren, authz_project).await?; - use db::schema::floating_ip::dsl; + + opctx.authorize(authz::Action::ListChildren, authz_project).await?; match pagparams { PaginatedBy::Id(pagparams) => { diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index a7a5e8a9217..78638e3c770 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -1747,7 +1747,9 @@ mod test { }; db::model::Name(Name::try_from(name).unwrap()) }); - // We do name duplicate checking on the `Some` branch. + + // We do name duplicate checking on the `Some` branch, don't steal the + // name intended for another floating IP. if parent_id.is_none() && modify_name { continue; } @@ -1794,21 +1796,20 @@ mod test { seen_pairs.insert(key); } else if !valid_expression { + // Several permutations are invalid and we want to detect them all. // NOTE: CHECK violation will supersede UNIQUE violation below. - // At least one is not valid, we expect a check violation let err = res.expect_err( "Expected a CHECK violation when inserting a \ - Floating IP record with NULL name and/or description", + Floating IP record with NULL name and/or description, \ + and incorrect project parent relation", ); assert!( matches!(err, DatabaseError(CheckViolation, _)), "Expected a CHECK violation when inserting a \ Floating IP record with NULL name and/or description, \ - and incorrect project parent relation \ - \nGOT: {err:?}", + and incorrect project parent relation", ); } else { - // At least one is not valid, we expect a check violation let err = res.expect_err( "Expected a UNIQUE violation when inserting a \ Floating IP record with existing (name, project_id)", @@ -1816,8 +1817,7 @@ mod test { assert!( matches!(err, DatabaseError(UniqueViolation, _)), "Expected a UNIQUE violation when inserting a \ - Floating IP record with existing (name, project_id) \ - \nGOT: {err:?}", + Floating IP record with existing (name, project_id)", ); } } diff --git a/nexus/src/app/instance.rs b/nexus/src/app/instance.rs index 76e9a0b4ffe..0edb2c5ea79 100644 --- a/nexus/src/app/instance.rs +++ b/nexus/src/app/instance.rs @@ -925,10 +925,11 @@ impl super::Nexus { .into_iter() .partition(|ip| ip.kind == IpKind::Ephemeral); - if ephemeral_ips.len() > 1 { + if ephemeral_ips.len() > MAX_EPHEMERAL_IPS_PER_INSTANCE { return Err(Error::internal_error( format!( - "Expected at most one ephemeral IP for an instance, found {}", + "Expected at most {} ephemeral IP for an instance, found {}", + MAX_EPHEMERAL_IPS_PER_INSTANCE, ephemeral_ips.len() ) .as_str(), diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs index e1c4f273a72..d1644a63804 100644 --- a/nexus/src/app/sagas/instance_create.rs +++ b/nexus/src/app/sagas/instance_create.rs @@ -619,8 +619,8 @@ async fn sic_allocate_instance_external_ip( let instance_id = repeat_saga_params.instance_id; let ip_id = repeat_saga_params.new_id; - // Collect the possible pool name for this IP address match ip_params { + // Allocate a new IP address from the target, possibly default, pool params::ExternalIpCreate::Ephemeral { ref pool_name } => { let pool_name = pool_name.as_ref().map(|name| db::model::Name(name.clone())); @@ -634,6 +634,7 @@ async fn sic_allocate_instance_external_ip( .await .map_err(ActionError::action_failed)?; } + // Set the parent of an existing floating IP to the new instance's ID. params::ExternalIpCreate::Floating { ref floating_ip_name } => { let floating_ip_name = db::model::Name(floating_ip_name.clone()); let (.., authz_fip, db_fip) = LookupPath::new(&opctx, &datastore) diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index dffff1a4728..f54370c32fb 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -4,9 +4,8 @@ //! Tests basic instance support in the API -use crate::integration_tests::external_ips::floating_ip_get; -use crate::integration_tests::external_ips::get_floating_ip_by_id_url; - +use super::external_ips::floating_ip_get; +use super::external_ips::get_floating_ip_by_id_url; use super::metrics::{get_latest_silo_metric, get_latest_system_metric}; use camino::Utf8Path; diff --git a/openapi/nexus.json b/openapi/nexus.json index 205d6380d4f..f0b66fbcc2a 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10637,7 +10637,7 @@ "properties": { "address": { "nullable": true, - "description": "An IP address to reserve for use as a floating IP. This field is optional if a pool is provided, in which case an address will be automatically chosen from there.", + "description": "An IP address to reserve for use as a floating IP. This field is optional: when not set, an address will be automatically chosen from `pool`. If set, then the IP must be available in the resolved `pool`.", "type": "string", "format": "ip" }, @@ -10649,7 +10649,7 @@ }, "pool": { "nullable": true, - "description": "The parent IP pool that a floating IP is pulled from. If combined with an explicit address, then that address must be available in the pool.", + "description": "The parent IP pool that a floating IP is pulled from. If unset, the default pool is selected.", "allOf": [ { "$ref": "#/components/schemas/NameOrId" From 649eba7dfff206cf681e0dbae6f673ef25045ec6 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 28 Nov 2023 19:47:32 +0000 Subject: [PATCH 21/27] Accidentally a cargo fmt --- nexus/db-queries/src/db/datastore/external_ip.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index 41e9f657d34..ba51eb482ba 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -385,7 +385,7 @@ impl DataStore { pagparams: &PaginatedBy<'_>, ) -> ListResultVec { use db::schema::floating_ip::dsl; - + opctx.authorize(authz::Action::ListChildren, authz_project).await?; match pagparams { From b29188e5b230c375694e502fee60166f526fef85 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 28 Nov 2023 22:58:51 +0000 Subject: [PATCH 22/27] Correct doc comment for openapi test. --- nexus/types/src/external_api/params.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 13433d0001f..f8091c1b252 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -767,14 +767,13 @@ pub struct FloatingIpCreate { #[serde(flatten)] pub identity: IdentityMetadataCreateParams, - /// An IP address to reserve for use as a floating IP. This field is - /// optional if a pool is provided, in which case an address will - /// be automatically chosen from there. + /// An IP address to reserve for use as a floating IP. This field is + /// optional: when not set, an address will be automatically chosen from + /// `pool`. If set, then the IP must be available in the resolved `pool`. pub address: Option, - /// The parent IP pool that a floating IP is pulled from. If combined - /// with an explicit address, then that address must be available in - /// the pool. + /// The parent IP pool that a floating IP is pulled from. If unset, the + /// default pool is selected. pub pool: Option, } From 9680ac8d5e986f10a8bd40ea6d251fcb16ea1085 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 1 Dec 2023 11:25:59 +0000 Subject: [PATCH 23/27] Use published OPTE. --- .github/buildomat/jobs/deploy.sh | 2 +- Cargo.lock | 12 ++++++------ Cargo.toml | 4 ++-- tools/opte_version | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/buildomat/jobs/deploy.sh b/.github/buildomat/jobs/deploy.sh index ff9b44fc40e..3c4b3d88c80 100755 --- a/.github/buildomat/jobs/deploy.sh +++ b/.github/buildomat/jobs/deploy.sh @@ -2,7 +2,7 @@ #: #: name = "helios / deploy" #: variety = "basic" -#: target = "lab-2.0-opte-0.25" +#: target = "lab-2.0-opte-0.27" #: output_rules = [ #: "%/var/svc/log/oxide-sled-agent:default.log*", #: "%/pool/ext/*/crypt/zone/oxz_*/root/var/svc/log/oxide-*.log*", diff --git a/Cargo.lock b/Cargo.lock index 31a44c45e1d..2272cdc2763 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3099,7 +3099,7 @@ dependencies = [ [[package]] name = "illumos-sys-hdrs" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" +source = "git+https://github.com/oxidecomputer/opte?rev=24ceba1969269e4d81bda83d8968d7d7f713c46b#24ceba1969269e4d81bda83d8968d7d7f713c46b" [[package]] name = "illumos-utils" @@ -3502,7 +3502,7 @@ dependencies = [ [[package]] name = "kstat-macro" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" +source = "git+https://github.com/oxidecomputer/opte?rev=24ceba1969269e4d81bda83d8968d7d7f713c46b#24ceba1969269e4d81bda83d8968d7d7f713c46b" dependencies = [ "quote", "syn 2.0.32", @@ -5182,7 +5182,7 @@ dependencies = [ [[package]] name = "opte" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" +source = "git+https://github.com/oxidecomputer/opte?rev=24ceba1969269e4d81bda83d8968d7d7f713c46b#24ceba1969269e4d81bda83d8968d7d7f713c46b" dependencies = [ "cfg-if", "dyn-clone", @@ -5198,7 +5198,7 @@ dependencies = [ [[package]] name = "opte-api" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" +source = "git+https://github.com/oxidecomputer/opte?rev=24ceba1969269e4d81bda83d8968d7d7f713c46b#24ceba1969269e4d81bda83d8968d7d7f713c46b" dependencies = [ "illumos-sys-hdrs", "ipnetwork", @@ -5210,7 +5210,7 @@ dependencies = [ [[package]] name = "opte-ioctl" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" +source = "git+https://github.com/oxidecomputer/opte?rev=24ceba1969269e4d81bda83d8968d7d7f713c46b#24ceba1969269e4d81bda83d8968d7d7f713c46b" dependencies = [ "libc", "libnet", @@ -5284,7 +5284,7 @@ dependencies = [ [[package]] name = "oxide-vpc" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/opte?rev=d508dcd12cbfeb8b948c0cffe800899046322233#d508dcd12cbfeb8b948c0cffe800899046322233" +source = "git+https://github.com/oxidecomputer/opte?rev=24ceba1969269e4d81bda83d8968d7d7f713c46b#24ceba1969269e4d81bda83d8968d7d7f713c46b" dependencies = [ "illumos-sys-hdrs", "opte", diff --git a/Cargo.toml b/Cargo.toml index aa7bc581041..05522f71f5d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -260,7 +260,7 @@ omicron-sled-agent = { path = "sled-agent" } omicron-test-utils = { path = "test-utils" } omicron-zone-package = "0.9.1" oxide-client = { path = "clients/oxide-client" } -oxide-vpc = { git = "https://github.com/oxidecomputer/opte", rev = "d508dcd12cbfeb8b948c0cffe800899046322233", features = [ "api", "std" ] } +oxide-vpc = { git = "https://github.com/oxidecomputer/opte", rev = "24ceba1969269e4d81bda83d8968d7d7f713c46b", features = [ "api", "std" ] } once_cell = "1.18.0" openapi-lint = { git = "https://github.com/oxidecomputer/openapi-lint", branch = "main" } openapiv3 = "1.0" @@ -268,7 +268,7 @@ openapiv3 = "1.0" openssl = "0.10" openssl-sys = "0.9" openssl-probe = "0.1.5" -opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "d508dcd12cbfeb8b948c0cffe800899046322233" } +opte-ioctl = { git = "https://github.com/oxidecomputer/opte", rev = "24ceba1969269e4d81bda83d8968d7d7f713c46b" } oso = "0.27" owo-colors = "3.5.0" oximeter = { path = "oximeter/oximeter" } diff --git a/tools/opte_version b/tools/opte_version index a8c459b83df..fa0ef8d7680 100644 --- a/tools/opte_version +++ b/tools/opte_version @@ -1 +1 @@ -0.27.217 +0.27.199 From 7fb49b801c52cc7225d7efc18717f375eb2df49a Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 1 Dec 2023 11:45:31 +0000 Subject: [PATCH 24/27] Missed a merge conflict. --- sled-agent/src/services.rs | 93 ++------------------------------------ 1 file changed, 5 insertions(+), 88 deletions(-) diff --git a/sled-agent/src/services.rs b/sled-agent/src/services.rs index 6b2d840a738..bd21d421094 100644 --- a/sled-agent/src/services.rs +++ b/sled-agent/src/services.rs @@ -1168,92 +1168,8 @@ impl ServiceManager { }) .collect(); -<<<<<<< HEAD - let mut ports = vec![]; - for svc in &req.services { - // Non-SNAT IPs are floating IPs with `is_service` true. - let external_ip; - let (nic, snat, floating_ips) = match &svc.details { - ServiceType::Nexus { external_ip, nic, .. } => { - (nic, None, std::slice::from_ref(external_ip)) - } - ServiceType::ExternalDns { dns_address, nic, .. } => { - external_ip = dns_address.ip(); - (nic, None, std::slice::from_ref(&external_ip)) - } - ServiceType::BoundaryNtp { nic, snat_cfg, .. } => { - (nic, Some(*snat_cfg), &[][..]) - } - _ => continue, - }; - - // Create the OPTE port for the service. - // Note we don't plumb any firewall rules at this point, - // Nexus will plumb them down later but the default OPTE - // config allows outbound access which is enough for - // Boundary NTP which needs to come up before Nexus. - let port = port_manager - .create_port( - nic, - snat, - None, - floating_ips, - &[], - DhcpCfg::default(), - ) - .map_err(|err| Error::ServicePortCreation { - service: svc.details.to_string(), - err: Box::new(err), - })?; - - // We also need to update the switch with the NAT mappings - // XXX: need to revisit iff. any services get more than one - // address. - let (target_ip, first_port, last_port) = match snat { - Some(s) => (s.ip, s.first_port, s.last_port), - None => (floating_ips[0], 0, u16::MAX), - }; - - for dpd_client in &dpd_clients { - // TODO-correctness(#2933): If we fail part-way we need to - // clean up previous entries instead of leaking them. - let nat_create = || async { - info!( - self.inner.log, "creating NAT entry for service"; - "service" => ?svc, - ); - - dpd_client - .ensure_nat_entry( - &self.inner.log, - target_ip.into(), - dpd_client::types::MacAddr { - a: port.0.mac().into_array(), - }, - first_port, - last_port, - port.0.vni().as_u32(), - underlay_address, - ) - .await - .map_err(BackoffError::transient)?; - - Ok::<(), BackoffError>>(()) - }; - let log_failure = |error, _| { - warn!( - self.inner.log, "failed to create NAT entry for service"; - "error" => ?error, - "service" => ?svc, - ); - }; - retry_notify( - retry_policy_internal_service_aggressive(), - nat_create, - log_failure, -======= let external_ip; - let (zone_type_str, nic, snat, external_ips) = match &zone_args + let (zone_type_str, nic, snat, floating_ips) = match &zone_args .omicron_type() { Some( @@ -1277,7 +1193,6 @@ impl ServiceManager { nic, None, std::slice::from_ref(&external_ip), ->>>>>>> main ) } Some( @@ -1294,16 +1209,18 @@ impl ServiceManager { // config allows outbound access which is enough for // Boundary NTP which needs to come up before Nexus. let port = port_manager - .create_port(nic, snat, external_ips, &[], DhcpCfg::default()) + .create_port(nic, snat, None, floating_ips, &[], DhcpCfg::default()) .map_err(|err| Error::ServicePortCreation { service: zone_type_str.clone(), err: Box::new(err), })?; // We also need to update the switch with the NAT mappings + // XXX: need to revisit iff. any services get more than one + // address. let (target_ip, first_port, last_port) = match snat { Some(s) => (s.ip, s.first_port, s.last_port), - None => (external_ips[0], 0, u16::MAX), + None => (floating_ips[0], 0, u16::MAX), }; for dpd_client in &dpd_clients { From 43ef99db9205e5583cd9b3af4fd0d792ca1de5d1 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Mon, 4 Dec 2023 17:33:13 +0000 Subject: [PATCH 25/27] Fix: detach floating IPs during create saga undo --- nexus/src/app/sagas/instance_create.rs | 38 +++++++++++++++++--------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs index d1644a63804..073dc97e7cc 100644 --- a/nexus/src/app/sagas/instance_create.rs +++ b/nexus/src/app/sagas/instance_create.rs @@ -605,25 +605,22 @@ async fn sic_allocate_instance_external_ip( let repeat_saga_params = sagactx.saga_params::()?; let saga_params = repeat_saga_params.saga_params; let ip_index = repeat_saga_params.which; - let ip_params = saga_params.create_params.external_ips.get(ip_index); - let ip_params = match ip_params { - None => { - return Ok(()); - } - Some(ref prs) => prs, + let Some(ip_params) = saga_params.create_params.external_ips.get(ip_index) + else { + return Ok(()); }; let opctx = crate::context::op_context_for_saga_action( &sagactx, &saga_params.serialized_authn, ); let instance_id = repeat_saga_params.instance_id; - let ip_id = repeat_saga_params.new_id; match ip_params { // Allocate a new IP address from the target, possibly default, pool params::ExternalIpCreate::Ephemeral { ref pool_name } => { let pool_name = pool_name.as_ref().map(|name| db::model::Name(name.clone())); + let ip_id = repeat_saga_params.new_id; datastore .allocate_instance_ephemeral_ip( &opctx, @@ -661,16 +658,31 @@ async fn sic_allocate_instance_external_ip_undo( let repeat_saga_params = sagactx.saga_params::()?; let saga_params = repeat_saga_params.saga_params; let ip_index = repeat_saga_params.which; - if ip_index >= saga_params.create_params.external_ips.len() { - return Ok(()); - } - let opctx = crate::context::op_context_for_saga_action( &sagactx, &saga_params.serialized_authn, ); - let ip_id = repeat_saga_params.new_id; - datastore.deallocate_external_ip(&opctx, ip_id).await?; + let Some(ip_params) = saga_params.create_params.external_ips.get(ip_index) + else { + return Ok(()); + }; + + match ip_params { + params::ExternalIpCreate::Ephemeral { .. } => { + let ip_id = repeat_saga_params.new_id; + datastore.deallocate_external_ip(&opctx, ip_id).await?; + } + params::ExternalIpCreate::Floating { floating_ip_name } => { + let floating_ip_name = db::model::Name(floating_ip_name.clone()); + let (.., authz_fip, db_fip) = LookupPath::new(&opctx, &datastore) + .project_id(saga_params.project_id) + .floating_ip_name(&floating_ip_name) + .fetch_for(authz::Action::Modify) + .await?; + + datastore.floating_ip_detach(&opctx, &authz_fip, &db_fip).await?; + } + } Ok(()) } From 49bd63488987e0022ed4eb98891d152335629b9a Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 6 Dec 2023 11:43:31 +0000 Subject: [PATCH 26/27] Minor fmt. --- nexus/db-queries/src/db/datastore/external_ip.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index 2d2b41fd247..e821082501c 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -210,8 +210,8 @@ impl DataStore { let name = data.name().clone(); let explicit_ip = data.explicit_ip().is_some(); NextExternalIp::new(data).get_result_async(conn).await.map_err(|e| { - use diesel::result::Error::NotFound; use diesel::result::Error::DatabaseError; + use diesel::result::Error::NotFound; match e { NotFound => { if explicit_ip { From 112e9f2a99dfeb33c07ac58bf44fe0f913f5ced1 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 6 Dec 2023 16:40:22 +0000 Subject: [PATCH 27/27] Review feedback: comment expansion. --- nexus/src/app/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nexus/src/app/mod.rs b/nexus/src/app/mod.rs index a3b2e963604..d4c2d596f80 100644 --- a/nexus/src/app/mod.rs +++ b/nexus/src/app/mod.rs @@ -81,6 +81,9 @@ pub(crate) const MAX_NICS_PER_INSTANCE: usize = 8; // XXX: Might want to recast as max *floating* IPs, we have at most one // ephemeral (so bounded in saga by design). +// The value here is arbitrary, but we need *a* limit for the instance +// create saga to have a bounded DAG. We might want to only enforce +// this during instance create (rather than live attach) in future. pub(crate) const MAX_EXTERNAL_IPS_PER_INSTANCE: usize = 32; pub(crate) const MAX_EPHEMERAL_IPS_PER_INSTANCE: usize = 1;