Compare commits

..

5 Commits

Author SHA1 Message Date
brynmailer-mdc 2961d73d03 test(compute): cover an unnamed boot device with no medium in the device map 2026-11-02 14:25:25 +10:00
brynmailer-mdc 042666b6ad refactor(compute): share the NBD node options and export dispatch
The data and boot -blockdev builders share one helper for the nbd server,
export and reconnect-delay options, buildDrives dispatches both branches
through one export helper, and hot attach parses its URI with the shared
parser. Emitted arguments are unchanged. The BlkDeviceMapped doc now states
when it falls back to the scalar form.
2026-11-02 14:25:25 +10:00
brynmailer-mdc 3a19e76f6b fix(compute): refuse a disk-boot launch with no boot volume
Data-volume -blockdev nodes satisfy Execute's drive check on their own, so
a disk-boot request list without a boot volume would start a QEMU with
nothing to boot. The disk-boot path now refuses it before launch.
2026-11-02 14:24:18 +10:00
brynmailer-mdc 03cb800f4c test(viperblock): pin the boot -blockdev shape in the nbdkit orphan reaper
The boot volume's NBD server options now carry a file. prefix, which the
referencer scan matches only because it compares by substring. Cover the
unix and TCP boot strings so a stricter match cannot reap a live root
export, and stop describing the joined host:port form as a boot drive.
2026-11-02 14:23:24 +10:00
brynmailer-mdc 6aa989e1ec feat(compute): boot the root volume from a -blockdev node that takes NBD or host_device
The boot volume moves from -drive if=none to a -blockdev node named os, so
a block export reaches QEMU as host_device as data volumes already do. The
NBD boot shape keeps its cache, reconnect, iothread and bootindex settings;
werror and rerror move onto the virtio-blk device. A block EFI volume is
still refused because pflash has no host_device form.
2026-11-02 14:14:07 +10:00
79 changed files with 606 additions and 2263 deletions
+2 -2
View File
@@ -126,8 +126,8 @@ type Assign struct {
ENIPrivateIP string `json:"eniPrivateIp,omitempty"`
ENISubnetID string `json:"eniSubnetId,omitempty"`
// TaskRoleARN is the task's IAM role (from the taskdef); when set, the agent
// serves its credentials at 169.254.170.2 under CredID, a scheduler-minted
// secret. An assign without a CredID gets no credentials.
// serves its credentials at 169.254.170.2 under CredID, defaulting CredID to
// the taskID when the scheduler leaves it empty.
CredID string `json:"credId,omitempty"`
TaskRoleARN string `json:"taskRoleArn,omitempty"`
// ExecutionRoleARN is the task's execution role (from the taskdef); when set,
+1 -20
View File
@@ -143,25 +143,6 @@ No user-facing operation replaces a control plane, and automatic replacement doe
Recreate the cluster to adopt it.
User-workload injection through a cluster created with an older image is not measured.
### Add-on versions
Add-on versions use the AWS form `v<x.y.z>-eksbuild.<n>`, for example `v1.40.1-eksbuild.1`.
`DescribeAddonVersions` returns versions in this form, newest first, and every other add-on response reports the version the same way.
Spinifex ships one EKS build of each upstream version, so `<n>` is `1` today.
`DescribeAddonVersions` honours `addonName`, `kubernetesVersion`, `types`, `publishers`, `owners`, `maxResults` and `nextToken`.
Each version carries `compatibilities` with the `clusterVersion`, the `platformVersions`, and a `defaultVersion` flag that is true for exactly one version per add-on and cluster version.
A `kubernetesVersion` that Spinifex does not run returns no add-ons.
`CreateAddon` and `UpdateAddon` accept the AWS form of a version in the catalogue.
They also accept the bare upstream form, such as `1.40.1`, and Spinifex logs each use at Info.
This is a Spinifex leniency that AWS does not offer, and it exists so that older scripts keep working.
Any other value is refused with a message that names `addonVersion` and lists the supported versions, for example `v1.40.1-eksbuild.2`, `1.40` or `v1.40.1`.
In Terraform, pass the AWS form to `aws_eks_addon.addon_version`, either as a literal or from `data.aws_eks_addon_version`.
The AWS provider validates the format before it calls the API, so it rejects the bare form.
Leaving `addon_version` unset still installs the catalogue default.
## Prerequisites
> [!IMPORTANT]
@@ -378,7 +359,7 @@ Control plane readiness and kubectl authentication above lists which clusters re
**`create-cluster` rejected with an authentication-mode error.** `CONFIG_MAP` is the one mode Spinifex rejects, because it has no `aws-auth` ConfigMap path. Set `authentication_mode` to `API` or `API_AND_CONFIG_MAP`, or leave it unset, and grant access with access entries.
**Addon install fails.** Only addons bundled into the registered `eks-node` image install successfully. If you pin an addon version in Terraform, use the AWS form from `DescribeAddonVersions` (see Add-on versions above), or leave `addon_version` unset so Spinifex selects the catalog default.
**Addon install fails.** Only addons bundled into the registered `eks-node` image install successfully. If you pin an addon version in Terraform, leave `addon_version` unset so Spinifex selects the catalog default.
## Reference Architectures
+1 -1
View File
@@ -20,7 +20,7 @@ There is no generation, so the record mixes desired fields with the latest obser
| Durable record | `clusters/{cluster}/addons/{addon}` holds `Record` (`domains/eks/addon/record.go`): `addonName`, `addonVersion`, `status`, `serviceAccountRoleArn`, `configurationValues`, `health`, `arn`, `tags`, `createdAt`, `modifiedAt` (the optional ones `omitempty`). `clusters/{cluster}/addons/{addon}/manifest` holds `Manifest` (`domains/eks/addon/record.go`): `addonName`, `addonVersion`, `serviceAccountRoleArn`, `configurationValues`. Listing skips the manifest sub-key by segment count. | `TestAddonPersistedLayout_RecordAndManifestKeysAndFieldNames`, `TestAddonRecord_DecodesHandWrittenJSON`, `domains/eks/addon/store_test.go` `TestRecord_RoundTrip`, `TestList_SkipsManifestSubKeys` |
| Writers | Record: the owner in `domains/eks/addon/owner.go`, reached from the `CreateAddon`, `UpdateAddon` and `DeleteAddon` handlers, the cluster reconciler's report relay and the GPU node group launch: `Owner.Create` (unconditional put), `Owner.Update` and its failure marking (revision CAS), `Owner.Delete` (get then delete), `Owner.ApplyReport` (revision CAS), `Owner.EnsureGPUDevicePlugin` through `Owner.Create`; and the cluster prefix purge (`DeleteClusterChildren`). Manifest: `StagingInstaller.Install` (unconditional put on create and update), `StagingInstaller.Uninstall` (`domains/eks/addon/delivery.go`), `Owner.Delete` and the prefix purge. | `TestAddonPersistedLayout_RecordAndManifestKeysAndFieldNames`, `TestUpdateAddon_ReportsSuccessfulWhileRecordIsUpdating`, `TestDeleteAddon_ReturnsDeletingAfterErasingRecordAndManifest`, `TestDeleteClusterChildren_SweepsAddonRecordsAndManifestsScopedToCluster`, `domains/eks/addon/store_test.go` `TestCasUpdate`, `TestDeleteRecord_RemovesRecordAndManifest` |
| Readers | `ListAddons`, `DescribeAddon`, `CreateAddon` (existence check) and the reconciler read the record. `ListStagedAddonManifests` reads every `/manifest` key for the cluster (`addon.ListManifests`); it is served on `eks.ListStagedAddonManifests` (`daemon/daemon.go`) and reached only through the gateway's internal-addons route. | `handlers/eks/addons_test.go` `TestAddon_DescribeListDelete`, `TestListStagedAddonManifests` |
| Catalogue | Static in-binary catalogue (`domains/eks/addon/catalog.go`): `aws-load-balancer-controller`, `argocd`, `aws-ebs-csi-driver`, and hidden-but-creatable `nvidia-device-plugin` and `spinifex-noop`; one version each, the first being the default. Versions are stored and used internally in the bare upstream form, and API responses show the AWS form `v<x.y.z>-eksbuild.<n>` (`Spec.AWSVersion`). Create and update accept the AWS form or the exact bare form and reject unknown names and other version strings with `InvalidParameterValue`, which the gateway returns as `InvalidParameterException`. | `domains/eks/addon/catalog_test.go`, `handlers/eks/addons_test.go` `TestDescribeAddonVersions_ReturnsCatalog`, `TestCreateAddon_UnknownAddonRejected`, `TestCreateAddon_UnknownVersionRejected`, `TestUpdateAddon_UnknownVersionRejected` |
| Catalogue | Static in-binary catalogue (`domains/eks/addon/catalog.go`): `aws-load-balancer-controller`, `argocd`, `aws-ebs-csi-driver`, and hidden-but-creatable `nvidia-device-plugin` and `spinifex-noop`; one version each, the first being the default. Create and update reject unknown names and versions with `InvalidParameterValue`. | `domains/eks/addon/catalog_test.go`, `handlers/eks/addons_test.go` `TestDescribeAddonVersions_ReturnsCatalog`, `TestCreateAddon_UnknownAddonRejected`, `TestCreateAddon_UnknownVersionRejected`, `TestUpdateAddon_UnknownVersionRejected` |
| Desired state | Version, `serviceAccountRoleArn`, `configurationValues` and create-time tags. `resolveConflicts`, `podIdentityAssociations` and `clientRequestToken` are accepted and dropped; `preserve` on delete is ignored. `configurationValues` reaches the guest but the agent discards it (`mulga-eks-addon-sync.sh` `sync_once`), so it has no effect. | `TestAddonPersistedLayout_RecordAndManifestKeysAndFieldNames`, `TestDeleteAddon_ReturnsDeletingAfterErasingRecordAndManifest` |
| Observed state | `status` and `health` are overwritten by guest reports on the same record as the desired fields. `health` is the latest failure message, projected as one `AddonIssue` with only `message`. Statuses held: `CREATING`, `ACTIVE`, `UPDATING`, `DEGRADED`, `CREATE_FAILED`; `DELETING` is defined but never stored. | `TestAddonRecord_DecodesHandWrittenJSON`, `handlers/eks/addon_status_reconciler_test.go` `TestApplyAddonStatusReport` |
| Generation | None. Reports carry `version` and `ts`, and neither is read: any report for the add-on name applies to whatever is currently desired. | `TestAddonStatusReport_MismatchedVersionIsAccepted` |
@@ -79,8 +79,7 @@ Terraform manages the cluster, the addons, the ACM cert, and the HTTPS Ingress;
- The `argocd` and `aws-ebs-csi-driver` bundles must be baked into the `eks-node` AMI — `describe-addon` returns `no baked bundle` otherwise.
- The EBS-CSI default StorageClass uses `provisioner: ebs.csi.aws.com`; a PVC against it provisions a **Viperblock-backed EBS volume**.
- If the PVC stays `Pending`, the app still serves (the counter falls back to in-memory) — see [Troubleshooting](#troubleshooting).
- Addon versions are unset, so Spinifex installs each catalog default.
To pin one, use the AWS form that `DescribeAddonVersions` returns, such as `v3.0.23-eksbuild.1`.
- Leave addon versions unset (the AWS provider and the catalog disagree on the version-string format).
- The Argo CD `Application` is a `kubernetes_manifest`, so the `argoproj.io` CRDs must exist before `apply` — apply the parent and let the addon reach `ACTIVE` first.
**Prerequisites:**
@@ -520,8 +520,8 @@ resource "aws_eks_node_group" "workers" {
#
# Spinifex wires the controller's AWS credentials and ELB-eligible subnets at the
# node level, so no IRSA role or subnet tagging is needed here. addon_version is
# omitted so Spinifex installs its catalog default. To pin one, use the AWS form
# from DescribeAddonVersions, for example v2.11.0-eksbuild.1.
# omitted: the AWS provider demands a v-prefixed version that the catalog rejects,
# so let Spinifex default to its catalog version.
# ---------------------------------------------------------------------------
resource "aws_eks_addon" "lbc" {
@@ -542,7 +542,7 @@ resource "aws_eks_addon" "lbc" {
# Installed through the EKS API; Spinifex stages the bundle host-side and the
# worker renders it into the K3s auto-deploy dir. The workloads module registers
# the git repo and creates the Argo CD Application that syncs the demo app.
# addon_version omitted: the catalog default is installed (see the LBC note).
# addon_version omitted (see the LBC note).
# ---------------------------------------------------------------------------
resource "aws_eks_addon" "argocd" {
@@ -562,7 +562,7 @@ resource "aws_eks_addon" "argocd" {
#
# Ships a default gp3 StorageClass (provisioner ebs.csi.aws.com). A PVC in the
# demo app's git manifests dynamically provisions a Viperblock-backed EBS volume
# so the app's state survives pod restarts. addon_version omitted: catalog default.
# so the app's state survives pod restarts. addon_version omitted (see LBC note).
# ---------------------------------------------------------------------------
resource "aws_eks_addon" "ebs_csi" {
@@ -74,8 +74,7 @@ The LBC is installed as a managed **addon** (`aws_eks_addon`). Spinifex wires th
- **`spinifex.io/managed-ingress = "false"`** disables the K3s built-in traefik/servicelb so the LBC owns ingress, matching AWS parity. Omit the tag (or set it `true`) and the built-in ingress stays on.
- The ALB targets the workers' **NodePort** (`target-type: instance`). The one SG rule admits the VPC CIDR on that port so the in-VPC ALB can reach the workers; the worker SG is otherwise auto-managed.
- Spinifex ACM supports **ImportCertificate only** — the cert is generated by the `tls` provider and imported, not requested.
- **`addon_version` is unset**, so Spinifex installs its catalog default.
To pin a version, use the AWS form that `DescribeAddonVersions` returns, such as `v2.11.0-eksbuild.1`, or read it from `data.aws_eks_addon_version`.
- **Leave `addon_version` unset** (see the [quickstart/addons notes](/docs/eks-quickstart)) — the AWS provider and the Spinifex catalog disagree on the version string format, so let the server choose.
**Prerequisites:**
@@ -446,8 +446,8 @@ resource "aws_eks_node_group" "workers" {
#
# Spinifex wires the controller's AWS credentials and ELB-eligible subnets at the
# node level, so no IRSA role or subnet tagging is needed here. addon_version is
# omitted so Spinifex installs its catalog default. To pin one, use the AWS form
# from DescribeAddonVersions, for example v2.11.0-eksbuild.1.
# omitted: the AWS provider demands a v-prefixed version that the catalog rejects,
# so let Spinifex default to its catalog version.
# ---------------------------------------------------------------------------
resource "aws_eks_addon" "lbc" {
+20 -27
View File
@@ -47,13 +47,6 @@ type credResponse struct {
// credEntry caches one task's assumed-role credentials alongside the role it was
// assumed from.
// credRole is the task role a credID serves and the task it belongs to; the
// task ID names the STS session, since the credID is a secret.
type credRole struct {
roleARN string
taskID string
}
type credEntry struct {
roleARN string
creds stsauth.Credentials
@@ -78,7 +71,7 @@ type credEndpoint struct {
assume func(ctx context.Context, roleARN, sessionName string) (stsauth.Credentials, error)
mu sync.Mutex
roles map[string]credRole // credID -> task role
roles map[string]string // credID -> taskRoleARN
cache map[string]credEntry // credID -> cached creds
httpClient *http.Client
@@ -105,7 +98,7 @@ func newCredEndpoint(provider credentials.CredentialsProvider, region, gatewayUR
port: port,
proxyPort: defaultCredProxyPort,
run: run,
roles: map[string]credRole{},
roles: map[string]string{},
cache: map[string]credEntry{},
}
c.assume = c.assumeOverGateway
@@ -114,12 +107,12 @@ func newCredEndpoint(provider credentials.CredentialsProvider, region, gatewayUR
// Register associates a credID with the task role it serves; Deregister removes
// it and drops any cached credentials. Both are no-ops for an empty credID.
func (c *credEndpoint) Register(credID, taskID, roleARN string) {
func (c *credEndpoint) Register(credID, roleARN string) {
if credID == "" || roleARN == "" {
return
}
c.mu.Lock()
c.roles[credID] = credRole{roleARN: roleARN, taskID: taskID}
c.roles[credID] = roleARN
c.mu.Unlock()
}
@@ -149,25 +142,25 @@ func (c *credEndpoint) ServeHTTP(w http.ResponseWriter, r *http.Request) {
http.Error(w, "not found", http.StatusNotFound)
return
}
creds, role, err := c.fetch(r.Context(), credID)
creds, roleARN, err := c.fetch(r.Context(), credID)
if err != nil {
if errors.Is(err, errUnknownCredID) {
http.Error(w, "not found", http.StatusNotFound)
return
}
slog.Error("ecs-agent: assume task role failed", "task", role.taskID, "role", role.roleARN, "err", err)
slog.Error("ecs-agent: assume task role failed", "credId", credID, "err", err)
http.Error(w, "credentials unavailable", http.StatusBadGateway)
return
}
w.Header().Set("Content-Type", "application/json")
if err := json.NewEncoder(w).Encode(credResponse{
RoleArn: role.roleARN,
RoleArn: roleARN,
AccessKeyID: creds.AccessKeyID,
SecretAccessKey: creds.SecretAccessKey,
Token: creds.SessionToken,
Expiration: creds.Expiration.UTC().Format(time.RFC3339),
}); err != nil {
slog.Warn("ecs-agent: encode credential response", "task", role.taskID, "err", err)
slog.Warn("ecs-agent: encode credential response", "credId", credID, "err", err)
}
}
@@ -176,28 +169,28 @@ var errUnknownCredID = fmt.Errorf("unknown credId")
// fetch returns valid credentials for credID, reusing the cache until within the
// refresh margin and otherwise assuming the task role over the gateway.
func (c *credEndpoint) fetch(ctx context.Context, credID string) (stsauth.Credentials, credRole, error) {
func (c *credEndpoint) fetch(ctx context.Context, credID string) (stsauth.Credentials, string, error) {
c.mu.Lock()
role, known := c.roles[credID]
roleARN, known := c.roles[credID]
if !known {
c.mu.Unlock()
return stsauth.Credentials{}, credRole{}, errUnknownCredID
return stsauth.Credentials{}, "", errUnknownCredID
}
if e, ok := c.cache[credID]; ok && e.roleARN == role.roleARN && credValid(e.creds, credRefreshMargin) {
if e, ok := c.cache[credID]; ok && e.roleARN == roleARN && credValid(e.creds, credRefreshMargin) {
c.mu.Unlock()
return e.creds, role, nil
return e.creds, roleARN, nil
}
c.mu.Unlock()
creds, err := c.assume(ctx, role.roleARN, sessionName(role.taskID))
creds, err := c.assume(ctx, roleARN, sessionName(credID))
if err != nil {
return stsauth.Credentials{}, role, err
return stsauth.Credentials{}, "", err
}
c.mu.Lock()
c.cache[credID] = credEntry{roleARN: role.roleARN, creds: creds}
c.cache[credID] = credEntry{roleARN: roleARN, creds: creds}
c.mu.Unlock()
return creds, role, nil
return creds, roleARN, nil
}
// AssumeProvider returns a CredentialsProvider that mints credentials by assuming
@@ -397,10 +390,10 @@ func credValid(c stsauth.Credentials, margin time.Duration) bool {
return time.Now().Add(margin).Before(c.Expiration)
}
// sessionName derives an AssumeRole RoleSessionName from the task ID, satisfying the
// sessionName derives an AssumeRole RoleSessionName from credID, satisfying the
// STS [\w+=,.@-]{2,64} constraint.
func sessionName(taskID string) string {
name := "ecs-" + taskID
func sessionName(credID string) string {
name := "ecs-" + credID
if len(name) > 64 {
name = name[:64]
}
+7 -27
View File
@@ -29,7 +29,7 @@ func TestCredEndpoint_ServesRegisteredCredID(t *testing.T) {
c, _ := newTestCredEndpoint(stsauth.Credentials{
AccessKeyID: "AKIA", SecretAccessKey: "secret", SessionToken: "tok", Expiration: exp,
})
c.Register("cred-1", "task-1", "arn:aws:iam::111122223333:role/task")
c.Register("cred-1", "arn:aws:iam::111122223333:role/task")
rr := httptest.NewRecorder()
c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/cred-1", nil))
@@ -51,26 +51,6 @@ func TestCredEndpoint_ServesRegisteredCredID(t *testing.T) {
}
}
// The STS session name lands in the assumed-role ARN and in STS logs, so it is
// built from the public task ID and never from the secret credID.
func TestCredEndpoint_SessionNameIsTaskIDNotCredID(t *testing.T) {
c, _ := newTestCredEndpoint(stsauth.Credentials{})
var gotSession string
c.assume = func(ctx context.Context, roleARN, session string) (stsauth.Credentials, error) {
gotSession = session
return stsauth.Credentials{AccessKeyID: "AKIA", SecretAccessKey: "secret"}, nil
}
c.Register("secret-cred", "task-1", "arn:role")
rr := httptest.NewRecorder()
c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/secret-cred", nil))
if rr.Code != http.StatusOK {
t.Fatalf("status = %d, want 200", rr.Code)
}
if gotSession != "ecs-task-1" {
t.Errorf("session name = %q, want ecs-task-1", gotSession)
}
}
func TestCredEndpoint_UnknownCredIDIs404(t *testing.T) {
c, calls := newTestCredEndpoint(stsauth.Credentials{AccessKeyID: "x", SecretAccessKey: "y"})
rr := httptest.NewRecorder()
@@ -85,7 +65,7 @@ func TestCredEndpoint_UnknownCredIDIs404(t *testing.T) {
func TestCredEndpoint_WrongMethodIs405(t *testing.T) {
c, _ := newTestCredEndpoint(stsauth.Credentials{AccessKeyID: "x", SecretAccessKey: "y"})
c.Register("cred-1", "task-1", "arn:role")
c.Register("cred-1", "arn:role")
rr := httptest.NewRecorder()
c.ServeHTTP(rr, httptest.NewRequest(http.MethodPost, "/v2/credentials/cred-1", nil))
if rr.Code != http.StatusMethodNotAllowed {
@@ -97,7 +77,7 @@ func TestCredEndpoint_CachesUntilExpiry(t *testing.T) {
c, calls := newTestCredEndpoint(stsauth.Credentials{
AccessKeyID: "AKIA", SecretAccessKey: "secret", Expiration: time.Now().Add(time.Hour),
})
c.Register("cred-1", "task-1", "arn:role")
c.Register("cred-1", "arn:role")
for hit := range 3 {
rr := httptest.NewRecorder()
c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/cred-1", nil))
@@ -115,7 +95,7 @@ func TestCredEndpoint_RefreshesNearExpiry(t *testing.T) {
c, calls := newTestCredEndpoint(stsauth.Credentials{
AccessKeyID: "AKIA", SecretAccessKey: "secret", Expiration: time.Now().Add(time.Minute),
})
c.Register("cred-1", "task-1", "arn:role")
c.Register("cred-1", "arn:role")
for range 2 {
rr := httptest.NewRecorder()
c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/cred-1", nil))
@@ -127,7 +107,7 @@ func TestCredEndpoint_RefreshesNearExpiry(t *testing.T) {
func TestCredEndpoint_DeregisterDropsCredID(t *testing.T) {
c, _ := newTestCredEndpoint(stsauth.Credentials{AccessKeyID: "x", SecretAccessKey: "y"})
c.Register("cred-1", "task-1", "arn:role")
c.Register("cred-1", "arn:role")
c.Deregister("cred-1")
rr := httptest.NewRecorder()
c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/cred-1", nil))
@@ -141,7 +121,7 @@ func TestCredEndpoint_AssumeFailureIs502(t *testing.T) {
c.assume = func(ctx context.Context, roleARN, sessionName string) (stsauth.Credentials, error) {
return stsauth.Credentials{}, context.DeadlineExceeded
}
c.Register("cred-1", "task-1", "arn:role")
c.Register("cred-1", "arn:role")
rr := httptest.NewRecorder()
c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, "/v2/credentials/cred-1", nil))
if rr.Code != http.StatusBadGateway {
@@ -153,7 +133,7 @@ func TestCredEndpoint_StartServesOverLoopback(t *testing.T) {
c, _ := newTestCredEndpoint(stsauth.Credentials{
AccessKeyID: "AKIA", SecretAccessKey: "secret", Expiration: time.Now().Add(time.Hour),
})
c.Register("cred-1", "task-1", "arn:role")
c.Register("cred-1", "arn:role")
c.port = 0 // bind an ephemeral port instead of privileged 80
if err := c.Start(); err != nil {
t.Fatalf("start: %v", err)
+1 -36
View File
@@ -1,12 +1,8 @@
package agent
import (
"net/http"
"net/http/httptest"
"testing"
"github.com/mulgadc/spinifex/internal/stsauth"
ecsv1 "github.com/mulgadc/spinifex/contracts/ecs/v1"
)
@@ -17,7 +13,7 @@ func TestTaskCredID(t *testing.T) {
want string
}{
{"no role", ecsv1.Assign{TaskID: "t1"}, ""},
{"role, no credID -> none", ecsv1.Assign{TaskID: "t1", TaskRoleARN: "arn:role"}, ""},
{"role, no credID -> taskID", ecsv1.Assign{TaskID: "t1", TaskRoleARN: "arn:role"}, "t1"},
{"role + credID", ecsv1.Assign{TaskID: "t1", CredID: "c9", TaskRoleARN: "arn:role"}, "c9"},
}
for _, tc := range cases {
@@ -27,37 +23,6 @@ func TestTaskCredID(t *testing.T) {
}
}
// The task ID is public (it is the task ARN suffix), so naming another task's ID
// at the credential endpoint must not yield that task's role.
func TestRegisterTaskCreds_TaskIDDoesNotServeCredentials(t *testing.T) {
c, _ := newTestCredEndpoint(stsauth.Credentials{AccessKeyID: "AKIA", SecretAccessKey: "secret"})
a := Agent{cred: c}
get := func(id string) int {
rr := httptest.NewRecorder()
c.ServeHTTP(rr, httptest.NewRequest(http.MethodGet, credRelativeURI(id), nil))
return rr.Code
}
legacy := ecsv1.Assign{TaskID: "victim-task", TaskRoleARN: "arn:aws:iam::000000000010:role/high"}
if got := a.registerTaskCreds(&legacy); got != "" {
t.Errorf("assign without CredID registered credID %q", got)
}
if code := get("victim-task"); code != http.StatusNotFound {
t.Errorf("GET by task ID of CredID-less assign = %d, want 404", code)
}
minted := ecsv1.Assign{TaskID: "victim-2", CredID: "secret-cred", TaskRoleARN: "arn:aws:iam::000000000010:role/high"}
if got := a.registerTaskCreds(&minted); got != "secret-cred" {
t.Fatalf("registerTaskCreds = %q, want secret-cred", got)
}
if code := get("victim-2"); code != http.StatusNotFound {
t.Errorf("GET by task ID = %d, want 404", code)
}
if code := get("secret-cred"); code != http.StatusOK {
t.Errorf("GET by CredID = %d, want 200", code)
}
}
func TestWithCredEnv(t *testing.T) {
base := map[string]string{"FOO": "bar"}
+1 -1
View File
@@ -63,7 +63,7 @@ func TestReconcile_AdoptsRunningLabeledContainer(t *testing.T) {
if !adopted["t-001"] {
t.Fatalf("t-001 not adopted; got %v", adopted)
}
if got := a.cred.roles["t-001"].roleARN; got != testTaskRole {
if got := a.cred.roles["t-001"]; got != testTaskRole {
t.Errorf("cred not re-registered: roles[t-001] = %q, want %q", got, testTaskRole)
}
if n := countStatus(cp.taskStates(), "t-001", ecsv1.TaskStatusRunning); n != 1 {
+7 -7
View File
@@ -306,24 +306,24 @@ func (a *Agent) pullResolver(as *ecsv1.Assign) ctrruntime.Resolver {
// credential endpoint and returns the credID (empty when the task has no role).
func (a *Agent) registerTaskCreds(as *ecsv1.Assign) string {
credID := taskCredID(as)
if credID == "" && as.TaskRoleARN != "" {
slog.Warn("ecs-agent: task has a role but no credential ID, starting it without credentials", "task", as.TaskID, "role", as.TaskRoleARN)
}
if credID == "" || a.cred == nil {
return credID
}
a.cred.Register(credID, as.TaskID, as.TaskRoleARN)
a.cred.Register(credID, as.TaskRoleARN)
return credID
}
// taskCredID is the credential ID a task's containers fetch credentials under:
// the scheduler-minted secret CredID. Empty when the task carries no IAM role or
// no CredID; the public task ID is never used, since any API caller can learn it.
// the scheduler-assigned CredID, falling back to the taskID. Empty when the task
// carries no IAM role.
func taskCredID(as *ecsv1.Assign) string {
if as.TaskRoleARN == "" {
return ""
}
return as.CredID
if as.CredID != "" {
return as.CredID
}
return as.TaskID
}
// withCredEnv returns env with AWS_CONTAINER_CREDENTIALS_RELATIVE_URI set for
-9
View File
@@ -134,9 +134,6 @@ func marshalEC2Response(action string, output any) ([]byte, error) {
var ec2Actions = map[string]ec2Action{
"DescribeInstances": ec2Handler(func(ctx context.Context, input *ec2.DescribeInstancesInput, c *call, accountID string) (any, error) {
if err := ec2instanceapi.ValidateDescribeInstancesInput(input); err != nil {
return nil, err
}
out, err := ec2instanceapi.DescribeInstancesChecked(ctx, input, c.NATS, 0, c.NodeIDs, accountID)
if err != nil {
return out, err
@@ -260,9 +257,6 @@ var ec2Actions = map[string]ec2Action{
}
}),
"DescribeImages": ec2Handler(func(ctx context.Context, input *ec2.DescribeImagesInput, c *call, accountID string) (any, error) {
if err := ec2imageapi.ValidateDescribeImagesInput(input); err != nil {
return nil, err
}
return ec2imageapi.DescribeImages(ctx, input, c.NATS, accountID)
}),
"CreateImage": ec2Handler(func(ctx context.Context, input *ec2.CreateImageInput, c *call, accountID string) (any, error) {
@@ -293,9 +287,6 @@ var ec2Actions = map[string]ec2Action{
return ec2zoneapi.DescribeAvailabilityZones(input, c.Region, c.AZ)
}),
"DescribeVolumes": ec2Handler(func(ctx context.Context, input *ec2.DescribeVolumesInput, c *call, accountID string) (any, error) {
if err := ec2volumeapi.ValidateDescribeVolumesInput(input); err != nil {
return nil, err
}
return ec2volumeapi.DescribeVolumes(ctx, input, c.NATS, accountID)
}),
"ModifyVolume": ec2Handler(func(ctx context.Context, input *ec2.ModifyVolumeInput, c *call, accountID string) (any, error) {
@@ -6,7 +6,6 @@ import (
"strings"
"testing"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -31,27 +30,3 @@ func TestImportKeyPair_Base64PaddingWorkaround(t *testing.T) {
assert.NotContains(t, q["PublicKeyMaterial"], "%3D",
"Expected no URL-encoded padding remaining")
}
// A well-prefixed ID of the wrong shape is refused before any service is
// asked, so it can never come back as NotFound.
func TestDescribe_MalformedIDRejectedBeforeLookup(t *testing.T) {
tests := []struct {
action, param, id, code, msg string
}{
{"DescribeInstances", "InstanceId.1", "i-zzz", awserrors.ErrorInvalidInstanceIDMalformed, `Invalid id: "i-zzz"`},
{"DescribeImages", "ImageId.1", "ami-zzz", awserrors.ErrorInvalidAMIIDMalformed, `Invalid id: "ami-zzz"`},
{"DescribeVolumes", "VolumeId.1", "vol-zzz", awserrors.ErrorInvalidParameterValue, "Value (vol-zzz) for parameter volumes is invalid. Expected: 'vol-...'."},
}
for _, tt := range tests {
t.Run(tt.action, func(t *testing.T) {
h := ec2Actions[tt.action]
input, err := h.parse(map[string]string{tt.param: tt.id})
require.NoError(t, err)
_, err = h.dispatch(t.Context(), tt.action, input, &call{Handler: NewHandler(Deps{})}, "123456789012")
code, msg, ok := awserrors.ResolveErrorDetail(err)
require.True(t, ok)
assert.Equal(t, tt.code, code)
assert.Equal(t, tt.msg, msg)
})
}
}
@@ -5,7 +5,6 @@ package image
import (
"context"
"errors"
"regexp"
"strings"
"github.com/aws/aws-sdk-go/service/ec2"
@@ -14,12 +13,8 @@ import (
"github.com/nats-io/nats.go"
)
// imageIDSuffixPattern is the AWS AMI ID shape after ami-: 8 (legacy) or 17
// lowercase hex characters.
var imageIDSuffixPattern = regexp.MustCompile(`^([0-9a-f]{8}|[0-9a-f]{17})$`)
// ValidateDescribeImagesInput accepts a nil input and rejects any ImageIds entry that is not
// ami- followed by 8 or 17 lowercase hex characters with InvalidAMIID.Malformed.
// ValidateDescribeImagesInput accepts a nil input and rejects any ImageIds entry without the ami-
// prefix with InvalidAMIID.Malformed.
func ValidateDescribeImagesInput(input *ec2.DescribeImagesInput) (err error) {
if input == nil {
return nil
@@ -27,16 +22,9 @@ func ValidateDescribeImagesInput(input *ec2.DescribeImagesInput) (err error) {
if input.ImageIds != nil {
for _, imageId := range input.ImageIds {
if imageId == nil {
continue
}
suffix, ok := strings.CutPrefix(*imageId, "ami-")
if !ok {
if imageId != nil && !strings.HasPrefix(*imageId, "ami-") {
return errors.New(awserrors.ErrorInvalidAMIIDMalformed)
}
if !imageIDSuffixPattern.MatchString(suffix) {
return awserrors.Errorf(awserrors.ErrorInvalidAMIIDMalformed, "Invalid id: %q", *imageId)
}
}
}
@@ -46,6 +34,11 @@ func ValidateDescribeImagesInput(input *ec2.DescribeImagesInput) (err error) {
// DescribeImages implements the EC2 DescribeImages action, returning the AMIs visible to
// accountID from the NATS image service.
func DescribeImages(ctx context.Context, input *ec2.DescribeImagesInput, natsConn *nats.Conn, accountID string) (output ec2.DescribeImagesOutput, err error) {
err = ValidateDescribeImagesInput(input)
if err != nil {
return output, err
}
imageService := ec2image.NewNATSImageService(natsConn, 0)
result, err := imageService.DescribeImages(ctx, input, accountID)
if err != nil {
@@ -53,8 +53,8 @@ func TestValidateDescribeImagesInput(t *testing.T) {
name: "MultipleValidImageIds",
input: &ec2.DescribeImagesInput{
ImageIds: []*string{
aws.String("ami-1a2b3c4d"),
aws.String("ami-0123456789abcdef1"),
aws.String("ami-111"),
aws.String("ami-222"),
},
},
wantErr: false,
@@ -63,7 +63,7 @@ func TestValidateDescribeImagesInput(t *testing.T) {
name: "MixedValidAndInvalidImageIds",
input: &ec2.DescribeImagesInput{
ImageIds: []*string{
aws.String("ami-0123456789abcdef0"),
aws.String("ami-valid"),
aws.String("invalid-id"),
},
},
@@ -85,24 +85,11 @@ func TestValidateDescribeImagesInput(t *testing.T) {
err := ValidateDescribeImagesInput(tt.input)
if tt.wantErr {
code, ok := awserrors.ResolveErrorCode(err)
assert.True(t, ok)
assert.Equal(t, tt.errMsg, code)
assert.Error(t, err)
assert.Equal(t, tt.errMsg, err.Error())
} else {
assert.NoError(t, err)
}
})
}
}
// AWS answers a well-prefixed ID whose suffix is not 8 or 17 lowercase hex
// characters with Malformed, not NotFound.
func TestValidateDescribeImagesInput_MalformedSuffix(t *testing.T) {
for _, id := range []string{"ami-zzz", "ami-0123456789ABCDEF0", "ami-0123456789abcdef", "ami-"} {
err := ValidateDescribeImagesInput(&ec2.DescribeImagesInput{ImageIds: []*string{aws.String(id)}})
code, msg, ok := awserrors.ResolveErrorDetail(err)
assert.True(t, ok, id)
assert.Equal(t, awserrors.ErrorInvalidAMIIDMalformed, code, id)
assert.Equal(t, `Invalid id: "`+id+`"`, msg, id)
}
}
@@ -39,6 +39,13 @@ func TestCreateImage_NilNATS(t *testing.T) {
assert.Error(t, err)
}
func TestDescribeImages_ValidationErrors(t *testing.T) {
_, err := DescribeImages(context.Background(), &ec2.DescribeImagesInput{
ImageIds: []*string{aws.String("bad-id")},
}, nil, "")
assert.EqualError(t, err, awserrors.ErrorInvalidAMIIDMalformed)
}
func TestDescribeImages_NilNATS(t *testing.T) {
_, err := DescribeImages(context.Background(), nil, nil, "acct-123")
assert.Error(t, err)
@@ -89,12 +89,6 @@ func DescribeInstances(ctx context.Context, input *ec2.DescribeInstancesInput, n
return &ec2.DescribeInstancesOutput{Reservations: reservations}, nil
}
// ValidateDescribeInstancesInput rejects any InstanceIds entry that is not a
// well-formed instance ID with InvalidInstanceID.Malformed.
func ValidateDescribeInstancesInput(input *ec2.DescribeInstancesInput) error {
return validateInstanceIDFormat(input.InstanceIds)
}
// DescribeInstancesChecked is the customer-facing variant served by the
// gateway's DescribeInstances action. On top of DescribeInstances' aggregation,
// it asserts InvalidInstanceID.NotFound for any explicitly named instance ID
@@ -25,7 +25,7 @@ var instanceIDPattern = regexp.MustCompile(`^i-([0-9a-f]{8}|[0-9a-f]{17})$`)
func validateInstanceIDFormat(ids []*string) error {
for _, id := range ids {
if id != nil && !instanceIDPattern.MatchString(*id) {
return awserrors.Errorf(awserrors.ErrorInvalidInstanceIDMalformed, "Invalid id: %q", *id)
return errors.New(awserrors.ErrorInvalidInstanceIDMalformed)
}
}
return nil
@@ -57,10 +57,8 @@ func TestValidateInstanceIDFormat(t *testing.T) {
}
for _, id := range malformed {
err := validateInstanceIDFormat([]*string{aws.String(id)})
code, msg, ok := awserrors.ResolveErrorDetail(err)
require.True(t, ok, id)
assert.Equal(t, awserrors.ErrorInvalidInstanceIDMalformed, code, id)
assert.Equal(t, `Invalid id: "`+id+`"`, msg, id)
require.Error(t, err, id)
assert.Equal(t, awserrors.ErrorInvalidInstanceIDMalformed, err.Error(), id)
}
err := validateInstanceIDFormat([]*string{aws.String("i-0123456789abcdef0"), aws.String("i-abc")})
require.Error(t, err, "one malformed id fails the whole batch")
@@ -102,9 +100,8 @@ func TestLifecycleVerbs_MalformedIDRejectedBeforeNATS(t *testing.T) {
}
for name, call := range calls {
err := call()
code, ok := awserrors.ResolveErrorCode(err)
require.True(t, ok, name)
assert.Equal(t, awserrors.ErrorInvalidInstanceIDMalformed, code, name)
require.Error(t, err, name)
assert.Equal(t, awserrors.ErrorInvalidInstanceIDMalformed, err.Error(), name)
}
require.NoError(t, nc.Flush())
assert.Zero(t, requests.Load(), "no NATS request may be sent for a batch holding a malformed id")
@@ -5,7 +5,6 @@ package volume
import (
"context"
"errors"
"regexp"
"strings"
"github.com/aws/aws-sdk-go/service/ec2"
@@ -14,13 +13,8 @@ import (
"github.com/nats-io/nats.go"
)
// volumeIDSuffixPattern is the AWS volume ID shape after vol-: 8 (legacy) or
// 17 lowercase hex characters.
var volumeIDSuffixPattern = regexp.MustCompile(`^([0-9a-f]{8}|[0-9a-f]{17})$`)
// ValidateDescribeVolumesInput accepts a nil input, rejects any VolumeIds entry without the
// vol- prefix with InvalidVolumeID.Malformed, and any with a malformed suffix with
// InvalidParameterValue, as AWS does.
// ValidateDescribeVolumesInput accepts a nil input and rejects any VolumeIds entry without the
// vol- prefix with InvalidVolumeID.Malformed.
func ValidateDescribeVolumesInput(input *ec2.DescribeVolumesInput) error {
if input == nil {
return nil
@@ -28,17 +22,9 @@ func ValidateDescribeVolumesInput(input *ec2.DescribeVolumesInput) error {
if input.VolumeIds != nil {
for _, volumeId := range input.VolumeIds {
if volumeId == nil {
continue
}
suffix, ok := strings.CutPrefix(*volumeId, "vol-")
if !ok {
if volumeId != nil && !strings.HasPrefix(*volumeId, "vol-") {
return errors.New(awserrors.ErrorInvalidVolumeIDMalformed)
}
if !volumeIDSuffixPattern.MatchString(suffix) {
return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
"Value (%s) for parameter volumes is invalid. Expected: 'vol-...'.", *volumeId)
}
}
}
@@ -49,6 +35,11 @@ func ValidateDescribeVolumesInput(input *ec2.DescribeVolumesInput) error {
func DescribeVolumes(ctx context.Context, input *ec2.DescribeVolumesInput, natsConn *nats.Conn, accountID string) (ec2.DescribeVolumesOutput, error) {
var output ec2.DescribeVolumesOutput
err := ValidateDescribeVolumesInput(input)
if err != nil {
return output, err
}
volumeService := ec2volume.NewNATSVolumeService(natsConn)
result, err := volumeService.DescribeVolumes(ctx, input, accountID)
if err != nil {
@@ -5,7 +5,6 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/stretchr/testify/assert"
)
@@ -37,9 +36,9 @@ func TestValidateDescribeVolumesInput(t *testing.T) {
name: "ValidMultipleVolumeIds",
input: &ec2.DescribeVolumesInput{
VolumeIds: []*string{
aws.String("vol-0123456789abcdef1"),
aws.String("vol-1a2b3c4d"),
aws.String("vol-0123456789abcdef2"),
aws.String("vol-abc123"),
aws.String("vol-def456"),
aws.String("vol-ghi789"),
},
},
wantErr: false,
@@ -72,7 +71,7 @@ func TestValidateDescribeVolumesInput(t *testing.T) {
name: "MixedValidAndInvalid",
input: &ec2.DescribeVolumesInput{
VolumeIds: []*string{
aws.String("vol-0123456789abcdef3"),
aws.String("vol-valid123"),
aws.String("invalid-id"),
},
},
@@ -82,7 +81,7 @@ func TestValidateDescribeVolumesInput(t *testing.T) {
{
name: "NilVolumeIdInList",
input: &ec2.DescribeVolumesInput{
VolumeIds: []*string{nil, aws.String("vol-0123456789abcdef1")},
VolumeIds: []*string{nil, aws.String("vol-abc123")},
},
wantErr: false,
},
@@ -101,15 +100,3 @@ func TestValidateDescribeVolumesInput(t *testing.T) {
})
}
}
// AWS answers a well-prefixed ID whose suffix is not 8 or 17 lowercase hex
// characters with InvalidParameterValue, not NotFound.
func TestValidateDescribeVolumesInput_MalformedSuffix(t *testing.T) {
for _, id := range []string{"vol-zzz", "vol-0123456789ABCDEF0", "vol-0123456789abcdef", "vol-"} {
err := ValidateDescribeVolumesInput(&ec2.DescribeVolumesInput{VolumeIds: []*string{aws.String(id)}})
code, msg, ok := awserrors.ResolveErrorDetail(err)
assert.True(t, ok, id)
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code, id)
assert.Equal(t, "Value ("+id+") for parameter volumes is invalid. Expected: 'vol-...'.", msg, id)
}
}
@@ -96,6 +96,13 @@ func TestModifyVolume_NilNATS(t *testing.T) {
assert.Error(t, err)
}
func TestDescribeVolumes_ValidationErrors(t *testing.T) {
_, err := DescribeVolumes(context.Background(), &ec2.DescribeVolumesInput{
VolumeIds: []*string{aws.String("bad-id")},
}, nil, "")
assert.EqualError(t, err, awserrors.ErrorInvalidVolumeIDMalformed)
}
func TestDescribeVolumes_NilNATS(t *testing.T) {
_, err := DescribeVolumes(context.Background(), nil, nil, "acct-123")
assert.Error(t, err)
+5 -22
View File
@@ -224,7 +224,7 @@ func (s *VPCServiceImpl) CreateNetworkInterface(ctx context.Context, input *ec2.
groupNames := make(map[string]string)
s.fillSGNames(ctx, accountID, record.SecurityGroupIds, groupNames)
return &ec2.CreateNetworkInterfaceOutput{
NetworkInterface: s.eniRecordToEC2(&record, accountID, groupNames, s.vpcDNSHostnames(ctx, accountID, record.VpcId)),
NetworkInterface: s.eniRecordToEC2(&record, accountID, groupNames),
}, nil
}
@@ -595,7 +595,7 @@ func (s *VPCServiceImpl) DescribeNetworkInterfaces(ctx context.Context, input *e
}
s.fillSGNames(ctx, accountID, record.SecurityGroupIds, groupNames)
enis = append(enis, s.eniRecordToEC2(&record, accountID, groupNames, s.vpcDNSHostnames(ctx, accountID, record.VpcId)))
enis = append(enis, s.eniRecordToEC2(&record, accountID, groupNames))
}
// If specific ENI IDs were requested but not found, return error
@@ -1076,26 +1076,9 @@ func (s *VPCServiceImpl) fillSGNames(ctx context.Context, accountID string, ids
}
}
// vpcDNSHostnames reports whether the VPC has enableDnsHostnames on. An
// unreadable VPC reads as off, so its interfaces go unnamed.
func (s *VPCServiceImpl) vpcDNSHostnames(ctx context.Context, accountID, vpcID string) bool {
entry, err := s.vpcKV.Get(ctx, kvutil.AccountKey(accountID, vpcID))
if err != nil {
slog.WarnContext(ctx, "vpcDNSHostnames: VPC read failed", "vpcId", vpcID, "err", err)
return false
}
var rec VPCRecord
if err := json.Unmarshal(entry.Value(), &rec); err != nil {
slog.WarnContext(ctx, "vpcDNSHostnames: VPC unmarshal failed", "vpcId", vpcID, "err", err)
return false
}
return rec.EnableDnsHostnames
}
// eniRecordToEC2 converts an ENI record to an EC2 NetworkInterface, naming
// each security group from groupNames. AWS names the interface only while its
// VPC has enableDnsHostnames on.
func (s *VPCServiceImpl) eniRecordToEC2(record *ENIRecord, accountID string, groupNames map[string]string, dnsHostnames bool) *ec2.NetworkInterface {
// each security group from groupNames.
func (s *VPCServiceImpl) eniRecordToEC2(record *ENIRecord, accountID string, groupNames map[string]string) *ec2.NetworkInterface {
// ENIs with spinifex:managed-by tag are system-managed (e.g. by ELBv2)
requesterManaged := record.Tags["spinifex:managed-by"] != ""
@@ -1133,7 +1116,7 @@ func (s *VPCServiceImpl) eniRecordToEC2(record *ENIRecord, accountID string, gro
eni.Groups = groups
}
if dnsHostnames && s.privateDNSName != nil && record.PrivateIpAddress != "" {
if s.privateDNSName != nil && record.PrivateIpAddress != "" {
if name := s.privateDNSName(record.PrivateIpAddress); name != "" {
eni.PrivateDnsName = aws.String(name)
eni.PrivateIpAddresses[0].PrivateDnsName = aws.String(name)
+13 -56
View File
@@ -55,11 +55,15 @@ func TestNetworkInterface_NamesGroupsAndPrivateAddress(t *testing.T) {
vpcId := createTestVPC(t, svc, "10.0.0.0/16")
subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
sgId := createTestSG(t, svc, vpcId, "web")
setHostnames(t, svc, vpcId, true)
// Without a namer an interface carries no private name.
unnamed := createTestENI(t, svc, subnetId)
assert.Nil(t, describeTestENI(t, svc, unnamed).PrivateDnsName)
desc, err := svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
NetworkInterfaceIds: []*string{aws.String(unnamed)},
}, testAccountID)
require.NoError(t, err)
require.Len(t, desc.NetworkInterfaces, 1)
assert.Nil(t, desc.NetworkInterfaces[0].PrivateDnsName)
svc.SetPrivateDNSNamer(func(ip string) string { return "name-of-" + ip })
out, err := svc.CreateNetworkInterface(context.Background(), &ec2.CreateNetworkInterfaceInput{
@@ -68,8 +72,13 @@ func TestNetworkInterface_NamesGroupsAndPrivateAddress(t *testing.T) {
}, testAccountID)
require.NoError(t, err)
desc := describeTestENI(t, svc, *out.NetworkInterface.NetworkInterfaceId)
for name, eni := range map[string]*ec2.NetworkInterface{"create": out.NetworkInterface, "describe": desc} {
desc, err = svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
NetworkInterfaceIds: []*string{out.NetworkInterface.NetworkInterfaceId},
}, testAccountID)
require.NoError(t, err)
require.Len(t, desc.NetworkInterfaces, 1)
for name, eni := range map[string]*ec2.NetworkInterface{"create": out.NetworkInterface, "describe": desc.NetworkInterfaces[0]} {
want := "name-of-" + aws.StringValue(eni.PrivateIpAddress)
assert.Equal(t, want, aws.StringValue(eni.PrivateDnsName), name)
require.Len(t, eni.PrivateIpAddresses, 1, name)
@@ -80,58 +89,6 @@ func TestNetworkInterface_NamesGroupsAndPrivateAddress(t *testing.T) {
}
}
func TestNetworkInterface_PrivateDnsNameFollowsVpcDnsHostnames(t *testing.T) {
t.Parallel()
svc := setupTestVPCService(t)
svc.SetPrivateDNSNamer(func(ip string) string { return "name-of-" + ip })
vpcId := createTestVPC(t, svc, "10.0.0.0/16")
subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24")
// A new VPC has enableDnsHostnames off, so its interfaces go unnamed.
out, err := svc.CreateNetworkInterface(context.Background(), &ec2.CreateNetworkInterfaceInput{
SubnetId: aws.String(subnetId),
}, testAccountID)
require.NoError(t, err)
eniId := *out.NetworkInterface.NetworkInterfaceId
for name, eni := range map[string]*ec2.NetworkInterface{"create": out.NetworkInterface, "describe": describeTestENI(t, svc, eniId)} {
assert.Nil(t, eni.PrivateDnsName, name)
require.Len(t, eni.PrivateIpAddresses, 1, name)
assert.Nil(t, eni.PrivateIpAddresses[0].PrivateDnsName, name)
}
// Turning the flag on names the existing interface.
setHostnames(t, svc, vpcId, true)
eni := describeTestENI(t, svc, eniId)
want := "name-of-" + aws.StringValue(eni.PrivateIpAddress)
assert.Equal(t, want, aws.StringValue(eni.PrivateDnsName))
assert.Equal(t, want, aws.StringValue(eni.PrivateIpAddresses[0].PrivateDnsName))
// And turning it off again drops the name.
setHostnames(t, svc, vpcId, false)
eni = describeTestENI(t, svc, eniId)
assert.Nil(t, eni.PrivateDnsName)
assert.Nil(t, eni.PrivateIpAddresses[0].PrivateDnsName)
}
func setHostnames(t *testing.T, svc *VPCServiceImpl, vpcId string, on bool) {
t.Helper()
_, err := svc.ModifyVpcAttribute(context.Background(), &ec2.ModifyVpcAttributeInput{
VpcId: aws.String(vpcId),
EnableDnsHostnames: &ec2.AttributeBooleanValue{Value: aws.Bool(on)},
}, testAccountID)
require.NoError(t, err)
}
func describeTestENI(t *testing.T, svc *VPCServiceImpl, eniId string) *ec2.NetworkInterface {
t.Helper()
desc, err := svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{
NetworkInterfaceIds: []*string{aws.String(eniId)},
}, testAccountID)
require.NoError(t, err)
require.Len(t, desc.NetworkInterfaces, 1)
return desc.NetworkInterfaces[0]
}
func TestCreateNetworkInterface_SequentialIPs(t *testing.T) {
t.Parallel()
svc := setupTestVPCService(t)
+5 -9
View File
@@ -272,15 +272,11 @@ type TaskRecord struct {
// Group / StartedBy mirror the AWS task fields. A service's tasks carry
// Group="service:{name}" so the reconciler counts them and the task-state
// hook resolves a RUNNING/STOPPED task back to its owning service.
Group string `json:"group,omitempty"`
StartedBy string `json:"startedBy,omitempty"`
TaskDefFamily string `json:"taskDefFamily"`
TaskDefRevision int `json:"taskDefRevision"`
TaskDefARN string `json:"taskDefArn"`
// CredID is the secret under which the task's containers fetch task-role
// credentials from the agent. Set only when the task has a role; it must
// never be projected into an API response or logged.
CredID string `json:"credId,omitempty"`
Group string `json:"group,omitempty"`
StartedBy string `json:"startedBy,omitempty"`
TaskDefFamily string `json:"taskDefFamily"`
TaskDefRevision int `json:"taskDefRevision"`
TaskDefARN string `json:"taskDefArn"`
ContainerInstanceID string `json:"containerInstanceId,omitempty"`
ContainerInstanceARN string `json:"containerInstanceArn,omitempty"`
DesiredStatus string `json:"desiredStatus"`
+3 -14
View File
@@ -288,7 +288,7 @@ func (s *Service) DescribeClusters(ctx context.Context, input *ecs.DescribeClust
names = []string{defaultCluster}
}
includeSettings := slices.Contains(awsStringSlice(input.Include), ecs.ClusterFieldSettings)
out := &ecs.DescribeClustersOutput{Clusters: []*ecs.Cluster{}, Failures: []*ecs.Failure{}}
out := &ecs.DescribeClustersOutput{}
for _, name := range names {
name = ClusterShortName(name)
var rec ClusterRecord
@@ -458,7 +458,7 @@ func (s *Service) RegisterTaskDefinition(ctx context.Context, input *ecs.Registe
if family == "" {
return nil, errors.New(awserrors.ErrorECSInvalidParameter)
}
if err := validateContainerDefs(input.ContainerDefinitions, aws.StringValue(input.Memory)); err != nil {
if err := validateContainerDefs(input.ContainerDefinitions); err != nil {
return nil, err
}
warnUnsupportedLogDrivers(ctx, family, input.ContainerDefinitions)
@@ -497,23 +497,12 @@ func (s *Service) RegisterTaskDefinition(ctx context.Context, input *ecs.Registe
// The test here is per value, not per field: an empty list or a false/unset
// flag requests nothing and is accepted, because there is nothing to enforce
// or refuse. Only a value that asks for the restriction is rejected.
//
// Ahead of those, AWS's own rules: every container needs an image, and without
// a task-level memory every container needs memory or memoryReservation.
func validateContainerDefs(defs []*ecs.ContainerDefinition, taskMemory string) error {
func validateContainerDefs(defs []*ecs.ContainerDefinition) error {
for _, c := range defs {
if c == nil {
continue
}
name := aws.StringValue(c.Name)
if aws.StringValue(c.Image) == "" {
return awserrors.Errorf(awserrors.ErrorECSClientException,
"Container.image should not be null or empty.")
}
if taskMemory == "" && c.Memory == nil && c.MemoryReservation == nil {
return awserrors.Errorf(awserrors.ErrorECSClientException,
"Invalid setting for container '%s'. At least one of 'memory' or 'memoryReservation' must be specified.", name)
}
if len(c.Secrets) > 0 {
return errors.New(awserrors.ErrorECSInvalidParameter)
}
-109
View File
@@ -2,9 +2,7 @@ package ecs
import (
"context"
"encoding/json"
"github.com/mulgadc/spinifex/spinifex/domains/ecs/taskdefinition"
"maps"
"strconv"
"testing"
"time"
@@ -304,89 +302,6 @@ func TestService_RunTask_AssignCarriesTaskRole(t *testing.T) {
assert.Equal(t, roleARN, poll.Assignments[0].TaskRoleARN)
}
// A role task's credential ID is a scheduler-minted secret: not the public task
// ID, distinct per task, stable on re-delivery, persisted on the task record and
// absent from every API response. A role-less task gets none.
func TestService_TaskCredIDIsMintedSecret(t *testing.T) {
ctx := context.Background()
svc, _ := newTestService(t)
_, err := svc.CreateCluster(ctx, &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID)
require.NoError(t, err)
_, err = svc.RegisterTaskDefinition(ctx, &ecs.RegisterTaskDefinitionInput{
Family: aws.String("victim"),
TaskRoleArn: aws.String("arn:aws:iam::123456789012:role/task-high"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"),
Cpu: aws.Int64(128), Memory: aws.Int64(256), Essential: aws.Bool(true),
}},
}, testAccountID)
require.NoError(t, err)
registerTaskDef(t, svc, "attacker", 128, 256)
registerInstance(t, svc, "web", "i-1", 1024, 2048)
run, err := svc.RunTask(ctx, &ecs.RunTaskInput{Cluster: aws.String("web"), TaskDefinition: aws.String("victim")}, testAccountID)
require.NoError(t, err)
start, err := svc.StartTask(ctx, &ecs.StartTaskInput{
Cluster: aws.String("web"), TaskDefinition: aws.String("victim"), ContainerInstances: []*string{aws.String("i-1")},
}, testAccountID)
require.NoError(t, err)
_, err = svc.StartTask(ctx, &ecs.StartTaskInput{
Cluster: aws.String("web"), TaskDefinition: aws.String("attacker"), ContainerInstances: []*string{aws.String("i-1")},
}, testAccountID)
require.NoError(t, err)
poll := func() map[string]ecsv1.Assign {
p, perr := svc.PollAssignments(ctx, &PollAssignmentsInput{Cluster: "web", ContainerInstance: "i-1"}, testAccountID)
require.NoError(t, perr)
byTask := map[string]ecsv1.Assign{}
for _, as := range p.Assignments {
byTask[as.TaskID] = as
}
return byTask
}
first, second := poll(), poll()
require.Len(t, first, 3)
kv, err := svc.bucket(ctx, testAccountID)
require.NoError(t, err)
var apiJSON []byte
for _, out := range []any{run, start} {
b, merr := json.Marshal(out)
require.NoError(t, merr)
apiJSON = append(apiJSON, b...)
}
var taskARNs []*string
for as := range maps.Values(first) {
taskARNs = append(taskARNs, aws.String(as.TaskARN))
}
desc, err := svc.DescribeTasks(ctx, &ecs.DescribeTasksInput{Cluster: aws.String("web"), Tasks: taskARNs}, testAccountID)
require.NoError(t, err)
b, err := json.Marshal(desc)
require.NoError(t, err)
apiJSON = append(apiJSON, b...)
seen := map[string]bool{}
for taskID, as := range first {
if as.TaskRoleARN == "" {
assert.Empty(t, as.CredID, "role-less task got a credential ID")
continue
}
require.NotEmpty(t, as.CredID)
assert.NotEqual(t, taskID, as.CredID)
assert.False(t, seen[as.CredID], "credential ID reused across tasks")
seen[as.CredID] = true
assert.Equal(t, as.CredID, second[taskID].CredID, "re-delivered assign changed credential ID")
var rec TaskRecord
found, gerr := getJSON(ctx, kv, TaskKey("web", taskID), &rec)
require.NoError(t, gerr)
require.True(t, found)
assert.Equal(t, as.CredID, rec.CredID)
assert.NotContains(t, string(apiJSON), as.CredID, "credential ID leaked in an API response")
}
assert.Len(t, seen, 2)
}
// PollAssignments is at-least-once: re-poll without ack re-delivers the assign;
// acking it (then STOPPED) drains the inbox so it is never re-delivered.
func TestService_PollAssignments_AckAndReclaim(t *testing.T) {
@@ -897,12 +812,6 @@ func TestService_ListTasks_EmptyIsPresentNotAbsent(t *testing.T) {
assert.Empty(t, out.TaskArns)
}
func TestService_ListTasks_ClusterNotFound(t *testing.T) {
svc, _ := newTestService(t)
_, err := svc.ListTasks(context.Background(), &ecs.ListTasksInput{Cluster: aws.String("ghost")}, testAccountID)
require.EqualError(t, err, awserrors.ErrorECSClusterNotFound)
}
func TestService_ListContainerInstances_EmptyIsPresentNotAbsent(t *testing.T) {
svc, _ := newTestService(t)
_, err := svc.CreateCluster(context.Background(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID)
@@ -920,21 +829,3 @@ func TestAccountIDFromBucket(t *testing.T) {
_, ok = accountIDFromBucket("some-other-bucket")
assert.False(t, ok)
}
func TestService_DescribeClusters_ListsAlwaysPresent(t *testing.T) {
svc, _ := newTestService(t)
_, err := svc.CreateCluster(context.Background(), &ecs.CreateClusterInput{ClusterName: aws.String("web")}, testAccountID)
require.NoError(t, err)
found, err := svc.DescribeClusters(context.Background(), &ecs.DescribeClustersInput{Clusters: []*string{aws.String("web")}}, testAccountID)
require.NoError(t, err)
require.Len(t, found.Clusters, 1)
assert.NotNil(t, found.Failures)
assert.Empty(t, found.Failures)
miss, err := svc.DescribeClusters(context.Background(), &ecs.DescribeClustersInput{Clusters: []*string{aws.String("nope")}}, testAccountID)
require.NoError(t, err)
require.Len(t, miss.Failures, 1)
assert.NotNil(t, miss.Clusters)
assert.Empty(t, miss.Clusters)
}
+2 -2
View File
@@ -313,7 +313,7 @@ func TestService_CreateService_ProjectsNetworkConfigurationAndManagedTags(t *tes
Family: aws.String("app"),
NetworkMode: aws.String(NetworkModeAwsvpc),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
}},
}, testAccountID)
require.NoError(t, err)
@@ -352,7 +352,7 @@ func TestService_UpdateService_ChangesNetworkConfiguration(t *testing.T) {
Family: aws.String("app"),
NetworkMode: aws.String(NetworkModeAwsvpc),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
}},
}, testAccountID)
require.NoError(t, err)
@@ -16,8 +16,6 @@ import (
// agent to reach anything other than PENDING.
func seedTasks(t *testing.T, svc *ecsdomain.Service, cluster string, recs ...*ecsdomain.TaskRecord) {
t.Helper()
_, err := svc.CreateCluster(t.Context(), &ecs.CreateClusterInput{ClusterName: aws.String(cluster)}, ecsdomain.TestAccountID)
require.NoError(t, err)
kv, err := svc.Bucket(t.Context(), ecsdomain.TestAccountID)
require.NoError(t, err)
for _, r := range recs {
+18 -57
View File
@@ -19,7 +19,7 @@ func TestRegisterTaskDefinition_RejectsSecrets(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
Secrets: []*ecs.Secret{{
Name: aws.String("DB_PASSWORD"), ValueFrom: aws.String("arn:aws:ssm:::parameter/db"),
}},
@@ -37,7 +37,7 @@ func TestRegisterTaskDefinition_AcceptsUnsupportedLogDriver(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
LogConfiguration: &ecs.LogConfiguration{
LogDriver: aws.String("awslogs"),
Options: map[string]*string{"awslogs-group": aws.String("/ecs/app")},
@@ -67,7 +67,7 @@ func TestRegisterTaskDefinition_EchoesRequiresCompatibilities(t *testing.T) {
Family: aws.String("app"),
RequiresCompatibilities: aws.StringSlice([]string{"EC2"}),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
}},
}, testAccountID)
require.NoError(t, err)
@@ -91,7 +91,7 @@ func TestRegisterTaskDefinition_EchoesRuntimePlatform(t *testing.T) {
OperatingSystemFamily: aws.String("LINUX"),
},
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
}},
}, testAccountID)
require.NoError(t, err)
@@ -113,7 +113,7 @@ func TestRegisterTaskDefinition_RuntimePlatformOmittedWhenUnset(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
}},
}, testAccountID)
require.NoError(t, err)
@@ -253,7 +253,7 @@ func TestRegisterTaskDefinition_EchoesRuntimeFields(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
User: aws.String("1000:1000"),
ReadonlyRootFilesystem: aws.Bool(true),
Privileged: aws.Bool(false),
@@ -310,7 +310,7 @@ func TestRegisterTaskDefinition_RuntimeFieldsOmittedWhenUnset(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
}},
}, testAccountID)
require.NoError(t, err)
@@ -347,7 +347,7 @@ func TestRegisterTaskDefinition_EchoesEmptyCollections(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
Environment: []*ecs.KeyValuePair{},
MountPoints: []*ecs.MountPoint{},
VolumesFrom: []*ecs.VolumeFrom{},
@@ -386,7 +386,7 @@ func TestRegisterTaskDefinition_FillsAWSDefaults(t *testing.T) {
Family: aws.String("app"),
NetworkMode: aws.String("awsvpc"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
PortMappings: []*ecs.PortMapping{{ContainerPort: aws.Int64(80)}},
}},
}, testAccountID)
@@ -420,7 +420,7 @@ func TestRegisterTaskDefinition_BridgeLeavesHostPortUnset(t *testing.T) {
Family: aws.String("app"),
NetworkMode: aws.String("bridge"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
PortMappings: []*ecs.PortMapping{{ContainerPort: aws.Int64(53), Protocol: aws.String("udp")}},
}},
}, testAccountID)
@@ -435,7 +435,7 @@ func TestRegisterTaskDefinition_AcceptsEmptyRuntimeFields(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
MountPoints: []*ecs.MountPoint{},
VolumesFrom: []*ecs.VolumeFrom{},
SystemControls: []*ecs.SystemControl{},
@@ -455,7 +455,7 @@ func TestRegisterTaskDefinition_RejectsMountPoints(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
MountPoints: []*ecs.MountPoint{{
SourceVolume: aws.String("data"), ContainerPath: aws.String("/data"),
}},
@@ -476,7 +476,7 @@ func TestRegisterTaskDefinition_RejectsVolumesFrom(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
VolumesFrom: []*ecs.VolumeFrom{{SourceContainer: aws.String("other")}},
}},
}, testAccountID)
@@ -494,7 +494,7 @@ func TestRegisterTaskDefinition_RejectsLinuxParametersDevices(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
LinuxParameters: &ecs.LinuxParameters{
Devices: []*ecs.Device{{HostPath: aws.String("/dev/fuse")}},
},
@@ -514,7 +514,7 @@ func TestRegisterTaskDefinition_RejectsInitProcessEnabled(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
LinuxParameters: &ecs.LinuxParameters{InitProcessEnabled: aws.Bool(true)},
}},
}, testAccountID)
@@ -533,7 +533,7 @@ func TestRegisterTaskDefinition_RejectsSharedMemoryAndTmpfs(t *testing.T) {
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("shm"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
LinuxParameters: &ecs.LinuxParameters{SharedMemorySize: aws.Int64(64)},
}},
}, testAccountID)
@@ -545,7 +545,7 @@ func TestRegisterTaskDefinition_RejectsSharedMemoryAndTmpfs(t *testing.T) {
_, err = svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("tmpfs"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
LinuxParameters: &ecs.LinuxParameters{
Tmpfs: []*ecs.Tmpfs{{ContainerPath: aws.String("/tmp"), Size: aws.Int64(64)}},
},
@@ -567,7 +567,7 @@ func TestRunTask_AssignCarriesRuntimeFields(t *testing.T) {
_, err = svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"),
ContainerDefinitions: []*ecs.ContainerDefinition{{
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true), Memory: aws.Int64(256),
Name: aws.String("app"), Image: aws.String("registry/app:1"), Essential: aws.Bool(true),
User: aws.String("1000"),
ReadonlyRootFilesystem: aws.Bool(true),
Privileged: aws.Bool(true),
@@ -608,42 +608,3 @@ func TestRunTask_AssignCarriesRuntimeFields(t *testing.T) {
assert.Equal(t, "net.core.somaxconn", ac.SystemControls[0].Namespace)
assert.Equal(t, []string{"SYS_PTRACE"}, ac.CapAdd)
}
// AWS rejects a container with no image before checking its memory, and
// without task-level memory requires memory or memoryReservation per container.
func TestRegisterTaskDefinition_RequiresImageAndMemory(t *testing.T) {
tests := []struct {
name string
taskMemory *string
container *ecs.ContainerDefinition
wantErr string
}{
{"no image or memory", nil, &ecs.ContainerDefinition{Name: aws.String("web")},
"Container.image should not be null or empty."},
{"empty image", aws.String("512"), &ecs.ContainerDefinition{Name: aws.String("web"), Image: aws.String("")},
"Container.image should not be null or empty."},
{"no memory", nil, &ecs.ContainerDefinition{Name: aws.String("web"), Image: aws.String("registry/web:1")},
"Invalid setting for container 'web'. At least one of 'memory' or 'memoryReservation' must be specified."},
{"memoryReservation only", nil, &ecs.ContainerDefinition{
Name: aws.String("web"), Image: aws.String("registry/web:1"), MemoryReservation: aws.Int64(128)}, ""},
{"task-level memory", aws.String("512"), &ecs.ContainerDefinition{
Name: aws.String("web"), Image: aws.String("registry/web:1")}, ""},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
svc, _ := newTestService(t)
_, err := svc.RegisterTaskDefinition(context.Background(), &ecs.RegisterTaskDefinitionInput{
Family: aws.String("app"), Memory: tc.taskMemory,
ContainerDefinitions: []*ecs.ContainerDefinition{tc.container},
}, testAccountID)
if tc.wantErr == "" {
require.NoError(t, err)
return
}
code, msg, ok := awserrors.ResolveErrorDetail(err)
require.True(t, ok)
assert.Equal(t, awserrors.ErrorECSClientException, code)
assert.Equal(t, tc.wantErr, msg)
})
}
}
-12
View File
@@ -237,9 +237,6 @@ func (s *Service) newTaskRecord(accountID, cluster, taskID string, td *taskdefin
GPU: gpu,
CreatedAt: now,
}
if td.TaskRoleArn != "" {
rec.CredID = uuid.NewV4().String()
}
for _, c := range td.Containers {
rec.Containers = append(rec.Containers, ContainerState{Name: c.Name, Status: TaskStatusPending})
}
@@ -260,7 +257,6 @@ func (s *Service) publishAssign(ctx context.Context, kv jetstream.KeyValue, acco
TaskDefFamily: td.Family,
TaskDefRevision: td.Revision,
TaskRoleARN: td.TaskRoleArn,
CredID: rec.CredID,
ExecutionRoleARN: td.ExecutionRoleArn,
ENIID: rec.ENIID,
ENIMacAddress: rec.ENIMacAddress,
@@ -307,14 +303,6 @@ func (s *Service) ListTasks(ctx context.Context, input *ecs.ListTasksInput, acco
if err != nil {
return nil, err
}
var clusterRec ClusterRecord
found, err := getJSON(ctx, kv, ClusterMetaKey(cluster), &clusterRec)
if err != nil {
return nil, err
}
if !found {
return nil, errors.New(awserrors.ErrorECSClusterNotFound)
}
keys, err := keysWithPrefix(ctx, kv, TasksPrefix(cluster))
if err != nil {
return nil, err
+10 -75
View File
@@ -4,14 +4,8 @@ import (
"fmt"
"maps"
"slices"
"strconv"
"strings"
)
// firstBuild is the EKS build number of every bundled version; it moves per
// version when Spinifex rebuilds a bundle.
const firstBuild = 1
// NvidiaDevicePlugin is staged by a GPU node group's launch (EnsureGPUDevicePlugin),
// never user-requested directly.
const NvidiaDevicePlugin = "nvidia-device-plugin"
@@ -24,11 +18,6 @@ type Spec struct {
// Versions lists supported versions newest-first; [0] is the default.
Versions []string
DefaultVersion string
// Builds maps each upstream version to its EKS build number, the n in
// v<upstream>-eksbuild.<n>.
Builds map[string]int
// Type is the AWS add-on type reported by DescribeAddonVersions.
Type string
// RequiresIRSA indicates the add-on needs an IAM role for service-account binding.
RequiresIRSA bool
Description string
@@ -39,26 +28,26 @@ type Spec struct {
// catalog is the bundled add-on registry. Keep newest version first per slice.
var catalog = buildCatalog(
typed("networking", newSpec("aws-load-balancer-controller", true,
newSpec("aws-load-balancer-controller", true,
"Provisions ELBv2 load balancers for Kubernetes Service/Ingress resources.",
"2.11.0")),
typed("gitops", newSpec("argocd", false,
"2.11.0"),
newSpec("argocd", false,
"Declarative GitOps continuous delivery for Kubernetes.",
"3.0.23")),
typed("storage", newSpec("aws-ebs-csi-driver", true,
"3.0.23"),
newSpec("aws-ebs-csi-driver", true,
"Container Storage Interface driver for Amazon EBS (Viperblock) volumes.",
"1.40.1")),
"1.40.1"),
// Staged automatically for GPU node groups, so hidden from the public
// catalogue like spinifex-noop but still creatable by name.
hidden(typed("accelerator", newSpec(NvidiaDevicePlugin, false,
hidden(newSpec(NvidiaDevicePlugin, false,
"NVIDIA device plugin exposing nvidia.com/gpu allocatable via CDI on GPU-tainted nodes.",
"0.17.4"))),
"0.17.4")),
// spinifex-noop is the delivery-transport fixture: a trivial bundle
// (Namespace + ConfigMap) the add-on e2e uses to prove stage, render,
// auto-deploy, ACTIVE and delete round-trip without a real workload.
hidden(typed("fixture", newSpec("spinifex-noop", false,
hidden(newSpec("spinifex-noop", false,
"No-op delivery-transport fixture (Namespace + ConfigMap).",
"0.1.0"))),
"0.1.0")),
)
// hidden marks a spec as internal: still creatable, but absent from the
@@ -68,12 +57,6 @@ func hidden(s Spec) Spec {
return s
}
// typed sets the AWS add-on type reported by DescribeAddonVersions.
func typed(addonType string, s Spec) Spec {
s.Type = addonType
return s
}
// newSpec builds a spec with the first version as its default.
func newSpec(name string, requiresIRSA bool, description string, versions ...string) Spec {
spec := Spec{
@@ -85,10 +68,6 @@ func newSpec(name string, requiresIRSA bool, description string, versions ...str
if len(versions) > 0 {
spec.DefaultVersion = versions[0]
}
spec.Builds = make(map[string]int, len(versions))
for _, v := range versions {
spec.Builds[v] = firstBuild
}
return spec
}
@@ -132,50 +111,6 @@ func (s Spec) SupportsVersion(version string) bool {
return slices.Contains(s.Versions, version)
}
// awsForm renders a catalog version as v<upstream>-eksbuild.<build>.
func (s Spec) awsForm(upstream string) string {
build, ok := s.Builds[upstream]
if !ok {
build = firstBuild
}
return "v" + upstream + "-eksbuild." + strconv.Itoa(build)
}
// AWSVersion returns the AWS form of a stored version. A version the catalogue
// does not list, or one already in AWS form, is returned unchanged.
func (s Spec) AWSVersion(stored string) string {
if !slices.Contains(s.Versions, stored) {
return stored
}
return s.awsForm(stored)
}
// AWSVersions returns every supported version in AWS form, newest first.
func (s Spec) AWSVersions() []string {
out := make([]string, len(s.Versions))
for i, v := range s.Versions {
out[i] = s.awsForm(v)
}
return out
}
// ParseVersion maps a client-supplied version, in AWS form or bare upstream
// form, to the upstream version stored on the record. Nothing else is accepted.
func (s Spec) ParseVersion(input string) (string, bool) {
if slices.Contains(s.Versions, input) {
return input, true
}
if !strings.HasPrefix(input, "v") {
return "", false
}
for _, v := range s.Versions {
if input == s.awsForm(v) {
return v, true
}
}
return "", false
}
// Specs returns every catalogue entry, hidden ones included, sorted by name.
func Specs() []Spec {
names := slices.Sorted(maps.Keys(catalog))
@@ -100,78 +100,3 @@ func TestValidateCatalog_ChecksTheBundledCatalog(t *testing.T) {
t.Cleanup(func() { catalog = previous })
require.EqualError(t, ValidateCatalog(), `add-on "broken" has no versions`)
}
// buildSpec is a two-version spec whose newest version is on EKS build 2, so
// a test cannot pass by hardcoding build 1.
func buildSpec() Spec {
return Spec{
Name: "fixture",
Versions: []string{"2.0.0", "1.0.0"},
Builds: map[string]int{"2.0.0": 2, "1.0.0": 1},
}
}
func TestSpec_AWSVersionUsesTheCatalogBuildNumber(t *testing.T) {
t.Parallel()
spec := buildSpec()
assert.Equal(t, "v2.0.0-eksbuild.2", spec.AWSVersion("2.0.0"))
assert.Equal(t, "v1.0.0-eksbuild.1", spec.AWSVersion("1.0.0"))
}
func TestSpec_AWSVersionIsIdempotent(t *testing.T) {
t.Parallel()
spec := buildSpec()
assert.Equal(t, "v2.0.0-eksbuild.2", spec.AWSVersion("v2.0.0-eksbuild.2"),
"a record already holding the AWS form must not be wrapped twice")
}
func TestSpec_AWSVersionsAreNewestFirst(t *testing.T) {
t.Parallel()
assert.Equal(t, []string{"v2.0.0-eksbuild.2", "v1.0.0-eksbuild.1"}, buildSpec().AWSVersions())
}
func TestSpec_ParseVersionAcceptsOnlyTheExactForms(t *testing.T) {
t.Parallel()
spec := buildSpec()
for input, want := range map[string]string{
"v2.0.0-eksbuild.2": "2.0.0",
"2.0.0": "2.0.0",
"v1.0.0-eksbuild.1": "1.0.0",
"1.0.0": "1.0.0",
} {
got, ok := spec.ParseVersion(input)
assert.True(t, ok, "%q must be accepted", input)
assert.Equal(t, want, got, "%q", input)
}
}
func TestSpec_ParseVersionRefusesEverythingElse(t *testing.T) {
t.Parallel()
spec := buildSpec()
for _, input := range []string{
"v2.0.0-eksbuild.1", // wrong build for 2.0.0
"v2.0.0-eksbuild.3",
"v1.0.0-eksbuild.2", // wrong build for 1.0.0
"v2.0.0", // missing build
"2.0", // partial
"v2.0.0-eksbuild.",
"2.0.0-eksbuild.2", // missing v
"V2.0.0-eksbuild.2",
" 2.0.0",
"9.9.9",
"",
} {
_, ok := spec.ParseVersion(input)
assert.False(t, ok, "%q must be refused", input)
}
}
func TestCatalog_EveryVersionHasAnExplicitBuild(t *testing.T) {
t.Parallel()
for _, spec := range Specs() {
for _, v := range spec.Versions {
assert.Equal(t, 1, spec.Builds[v], "%s %s needs an explicit build number", spec.Name, v)
assert.Equal(t, "v"+v+"-eksbuild.1", spec.AWSVersion(v), "%s %s", spec.Name, v)
}
}
}
@@ -1,105 +0,0 @@
package awsapi
import (
"context"
"encoding/json"
"sync"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/eks"
"github.com/mulgadc/spinifex/internal/testkit"
"github.com/nats-io/nats.go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// captureDescribeAddonVersions answers eks.DescribeAddonVersions and returns a
// function reading the last input the gateway sent to the service.
func captureDescribeAddonVersions(t *testing.T, nc *nats.Conn) func() eks.DescribeAddonVersionsInput {
t.Helper()
var (
mu sync.Mutex
last eks.DescribeAddonVersionsInput
)
sub, err := nc.Subscribe("eks.DescribeAddonVersions", func(m *nats.Msg) {
mu.Lock()
last = eks.DescribeAddonVersionsInput{}
_ = json.Unmarshal(m.Data, &last)
mu.Unlock()
_ = m.Respond([]byte(`{}`))
})
require.NoError(t, err)
t.Cleanup(func() { _ = sub.Unsubscribe() })
return func() eks.DescribeAddonVersionsInput {
mu.Lock()
defer mu.Unlock()
return last
}
}
// The gateway folds a GET query string into the body as a JSON object of
// string lists; the model binds every field below from the query.
func TestDescribeAddonVersions_PassesEveryModelQueryParameter(t *testing.T) {
_, nc := testutil.StartTestNATS(t)
got := captureDescribeAddonVersions(t, nc)
query := []byte(`{"addonName":["aws-ebs-csi-driver"],"kubernetesVersion":["1.32"],` +
`"maxResults":["2"],"nextToken":["tok"],"types":["storage","networking"],` +
`"publishers":["eks"],"owners":["aws"]}`)
_, err := DescribeAddonVersions(context.Background(), nc, acct, query)
require.NoError(t, err)
in := got()
assert.Equal(t, "aws-ebs-csi-driver", aws.StringValue(in.AddonName))
assert.Equal(t, "1.32", aws.StringValue(in.KubernetesVersion))
assert.Equal(t, int64(2), aws.Int64Value(in.MaxResults))
assert.Equal(t, "tok", aws.StringValue(in.NextToken))
assert.Equal(t, []string{"storage", "networking"}, aws.StringValueSlice(in.Types))
assert.Equal(t, []string{"eks"}, aws.StringValueSlice(in.Publishers))
assert.Equal(t, []string{"aws"}, aws.StringValueSlice(in.Owners))
}
func TestDescribeAddonVersions_NoQueryLeavesEveryFilterUnset(t *testing.T) {
_, nc := testutil.StartTestNATS(t)
got := captureDescribeAddonVersions(t, nc)
_, err := DescribeAddonVersions(context.Background(), nc, acct, nil)
require.NoError(t, err)
in := got()
assert.Nil(t, in.AddonName)
assert.Nil(t, in.KubernetesVersion)
assert.Nil(t, in.MaxResults)
assert.Nil(t, in.NextToken)
assert.Empty(t, in.Types)
}
func TestDescribeAddonVersions_RefusesMalformedQuery(t *testing.T) {
_, nc := testutil.StartTestNATS(t)
captureDescribeAddonVersions(t, nc)
_, err := DescribeAddonVersions(context.Background(), nc, acct, []byte(`{"maxResults":["many"]}`))
require.Error(t, err)
assert.Contains(t, err.Error(), "maxResults")
_, err = DescribeAddonVersions(context.Background(), nc, acct, []byte(`not json`))
require.Error(t, err)
}
// The route must hand the folded query to the wrapper, not drop it.
func TestDescribeAddonVersionsRoute_PassesTheQuery(t *testing.T) {
_, nc := testutil.StartTestNATS(t)
got := captureDescribeAddonVersions(t, nc)
for _, r := range routes {
if r.Action != "DescribeAddonVersions" {
continue
}
_, err := r.Handler(context.Background(), nc, acct, "", nil, []byte(`{"addonName":["argocd"]}`))
require.NoError(t, err)
assert.Equal(t, "argocd", aws.StringValue(got().AddonName))
return
}
t.Fatal("no DescribeAddonVersions route")
}
+2 -17
View File
@@ -16,23 +16,8 @@ func ListAddons(ctx context.Context, natsConn *nats.Conn, accountID, cluster str
}
// DescribeAddonVersions — GET /addons/supported-versions.
func DescribeAddonVersions(ctx context.Context, natsConn *nats.Conn, accountID string, query []byte) (*eks.DescribeAddonVersionsOutput, error) {
q, err := parseListQuery(query)
if err != nil {
return nil, err
}
input := &eks.DescribeAddonVersionsInput{
AddonName: q.str("addonName"),
KubernetesVersion: q.str("kubernetesVersion"),
NextToken: q.str("nextToken"),
Types: q.strs("types"),
Publishers: q.strs("publishers"),
Owners: q.strs("owners"),
}
if input.MaxResults, err = q.int64("maxResults"); err != nil {
return nil, err
}
return ekscluster.NewNATSEKSService(natsConn).DescribeAddonVersions(ctx, input, accountID)
func DescribeAddonVersions(ctx context.Context, natsConn *nats.Conn, accountID string) (*eks.DescribeAddonVersionsOutput, error) {
return ekscluster.NewNATSEKSService(natsConn).DescribeAddonVersions(ctx, &eks.DescribeAddonVersionsInput{}, accountID)
}
// CreateAddon — POST /clusters/{name}/addons.
-61
View File
@@ -1,61 +0,0 @@
package awsapi
import (
"encoding/json"
"strconv"
"github.com/aws/aws-sdk-go/aws"
awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
)
// listQuery holds the query-string inputs of a GET action, which the gateway
// folds into the body as a JSON object of string lists.
type listQuery map[string][]string
// parseListQuery decodes the folded query of a GET action. An empty body means
// the request carried no query string.
func parseListQuery(body []byte) (listQuery, error) {
q := listQuery{}
if len(body) == 0 {
return q, nil
}
if err := json.Unmarshal(body, (*map[string][]string)(&q)); err != nil {
return nil, awserrors.Errorf(awserrors.ErrorEKSInvalidParameter, "malformed query string")
}
return q, nil
}
// str returns the first value of name, or nil when it is absent or empty.
func (q listQuery) str(name string) *string {
if v := q[name]; len(v) > 0 && v[0] != "" {
return aws.String(v[0])
}
return nil
}
// strs returns every non-empty value of name, or nil when there are none.
func (q listQuery) strs(name string) []*string {
var out []string
for _, v := range q[name] {
if v != "" {
out = append(out, v)
}
}
if len(out) == 0 {
return nil
}
return aws.StringSlice(out)
}
// int64 returns the first value of name as an integer, or nil when absent.
func (q listQuery) int64(name string) (*int64, error) {
s := q.str(name)
if s == nil {
return nil, nil
}
n, err := strconv.ParseInt(*s, 10, 64)
if err != nil {
return nil, awserrors.Errorf(awserrors.ErrorEKSInvalidParameter, "%s must be an integer", name)
}
return aws.Int64(n), nil
}
+3 -25
View File
@@ -82,39 +82,17 @@ func (d Deps) dispatch(w http.ResponseWriter, inv dispatch.Invocation) error {
if err == nil {
return nil
}
action, _, _, routed := router.Lookup(inv.Request.Method, inv.Request.URL.EscapedPath())
if routed && IsInternalAction(action) {
if action, _, _, ok := router.Lookup(inv.Request.Method, inv.Request.URL.EscapedPath()); ok && IsInternalAction(action) {
return err
}
code, msg, ok := awserrors.ResolveErrorDetail(err)
if !ok || code != awserrors.ErrorInvalidParameterValue {
return err
}
wire := awserrors.ErrorEKSInvalidParameter
if override, ok := parameterRefusalCodes[action]; ok {
wire = override
if msg == "" {
msg = "a parameter of the request is not valid"
}
}
if msg == "" {
return errors.New(wire)
return errors.New(awserrors.ErrorEKSInvalidParameter)
}
return awserrors.Errorf(wire, "%s", msg)
}
// parameterRefusalCodes holds the operations whose model does not declare
// InvalidParameterException, with the declared code a refused parameter uses.
var parameterRefusalCodes = map[string]string{
"DescribeCluster": awserrors.ErrorEKSClient,
"DeleteCluster": awserrors.ErrorEKSClient,
"DescribeAccessEntry": awserrors.ErrorEKSInvalidRequest,
"DeleteAccessEntry": awserrors.ErrorEKSInvalidRequest,
"DisassociateAccessPolicy": awserrors.ErrorEKSInvalidRequest,
"ListAssociatedAccessPolicies": awserrors.ErrorEKSInvalidRequest,
"TagResource": awserrors.ErrorEKSBadRequest,
"UntagResource": awserrors.ErrorEKSBadRequest,
"ListTagsForResource": awserrors.ErrorEKSBadRequest,
return awserrors.Errorf(awserrors.ErrorEKSInvalidParameter, "%s", msg)
}
// serve resolves the route, authorizes the request, then runs the action.
@@ -1,98 +0,0 @@
//test:in-package — routes is unexported, and the table must cover exactly the
// actions the registration serves.
package awsapi
import (
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"github.com/mulgadc/bluebottle/pkg/iampolicy"
"github.com/mulgadc/spinifex/internal/awsmodel"
"github.com/mulgadc/spinifex/internal/testkit"
awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/mulgadc/spinifex/spinifex/ingress/aws/dispatch"
"github.com/nats-io/nats.go"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// routePath fills a route pattern with placeholder segments. The tag routes
// take an ARN, which has to be escaped to stay one path segment.
func routePath(pattern string) string {
arn := url.PathEscape("arn:aws:eks:us-east-1:111122223333:cluster/c")
path := strings.ReplaceAll(pattern, "*", arn)
for {
start := strings.Index(path, "{")
if start < 0 {
return path
}
end := strings.Index(path[start:], "}")
path = path[:start] + "x" + path[start+end+1:]
}
}
// A parameter refusal must reach the wire under a code the operation declares
// in the EKS model, or SDK clients cannot match it to a typed error.
func TestRegistration_ParameterRefusalCodeIsDeclaredByOperation(t *testing.T) {
_, nc := testutil.StartTestNATS(t)
sub, err := nc.Subscribe("eks.>", func(m *nats.Msg) {
if m.Reply != "" {
_ = m.Respond(awserrors.GenerateErrorPayload(awserrors.ErrorInvalidParameterValue))
}
})
require.NoError(t, err)
t.Cleanup(func() { _ = sub.Unsubscribe() })
model, err := awsmodel.Load(awsmodel.EKS)
require.NoError(t, err)
registration := NewRegistration(Deps{NATS: nc})
for _, route := range routes {
if IsInternalAction(route.Action) {
continue
}
// The handler takes no input and the model declares no client error.
if route.Action == "ListAccessPolicies" {
continue
}
t.Run(route.Action, func(t *testing.T) {
operation, ok := model.Operation(route.Action)
require.True(t, ok, "%s is not an operation in the EKS model", route.Action)
declared := make([]string, 0, len(operation.Errors))
for _, ref := range operation.Errors {
declared = append(declared, ref.Shape)
}
req := httptest.NewRequest(route.Method, routePath(route.Pattern), strings.NewReader(`{}`))
inv := dispatch.Invocation{
Request: req,
AccountID: "111122223333",
Region: "us-east-1",
Authorize: func(string, string, []string, iampolicy.ConditionKeys) error { return nil },
Actor: func() (dispatch.Actor, error) {
return dispatch.Actor{AccountID: "111122223333", PrincipalType: "user", UserName: "alice"}, nil
},
}
err := registration.Dispatch(httptest.NewRecorder(), inv)
require.Error(t, err)
code, ok := awserrors.ResolveErrorCode(err)
require.True(t, ok, "unresolvable error code: %v", err)
assert.Contains(t, declared, code,
"%s %s refuses a parameter with %s, which the operation does not declare", route.Method, route.Pattern, code)
})
}
}
// The access operations declare InvalidRequestException, not a parameter code.
func TestEKSInvalidRequestCodeIsRegistered(t *testing.T) {
assert.Equal(t, "InvalidRequestException", awserrors.ErrorEKSInvalidRequest)
assert.Equal(t, http.StatusBadRequest, awserrors.HTTPStatusForError(awserrors.Errorf(awserrors.ErrorEKSInvalidRequest, "x")))
assert.Equal(t, "BadRequestException", awserrors.ErrorEKSBadRequest)
assert.Equal(t, http.StatusBadRequest, awserrors.HTTPStatusForError(awserrors.Errorf(awserrors.ErrorEKSBadRequest, "x")))
}
+1 -1
View File
@@ -131,7 +131,7 @@ var routes = []route{
// Addons
{Method: "GET", Pattern: "/addons/supported-versions", Action: "DescribeAddonVersions",
Handler: func(ctx context.Context, nc *nats.Conn, acct, callerARN string, p []string, b []byte) (any, error) {
return DescribeAddonVersions(ctx, nc, acct, b)
return DescribeAddonVersions(ctx, nc, acct)
}},
{Method: "GET", Pattern: "/clusters/{clusterName}/addons", Action: "ListAddons",
Handler: func(ctx context.Context, nc *nats.Conn, acct, callerARN string, p []string, b []byte) (any, error) {
+1 -1
View File
@@ -176,7 +176,7 @@ func TestGatewayWrappers_Addons(t *testing.T) {
require.NoError(t, err)
assert.NotNil(t, out1)
out2, err := DescribeAddonVersions(context.Background(), nc, acct, nil)
out2, err := DescribeAddonVersions(context.Background(), nc, acct)
require.NoError(t, err)
assert.NotNil(t, out2)
@@ -1,171 +0,0 @@
package cluster
import (
"context"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/eks"
"github.com/mulgadc/spinifex/spinifex/domains/eks/addon"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func describeVersions(t *testing.T, svc *EKSServiceImpl, in *eks.DescribeAddonVersionsInput) *eks.DescribeAddonVersionsOutput {
t.Helper()
out, err := svc.DescribeAddonVersions(context.Background(), in, testAccountID)
require.NoError(t, err)
return out
}
// The terraform aws_eks_addon_version data source asks for one add-on at the
// cluster's Kubernetes version, then takes the version whose compatibility for
// that cluster version is the default (or, with most_recent, the first one).
func TestDescribeAddonVersions_TerraformProviderCall(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
out := describeVersions(t, svc, &eks.DescribeAddonVersionsInput{
AddonName: aws.String(csiAddon),
KubernetesVersion: aws.String(defaultK8sVersion),
})
require.Len(t, out.Addons, 1)
info := out.Addons[0]
assert.Equal(t, csiAddon, aws.StringValue(info.AddonName))
assert.Nil(t, out.NextToken)
spec, ok := addon.Lookup(csiAddon)
require.True(t, ok)
require.Equal(t, spec.AWSVersions(), versionStrings(info), "newest first, AWS form")
defaults := 0
for _, v := range info.AddonVersions {
require.Len(t, v.Compatibilities, 1, "one entry, for the requested cluster version")
c := v.Compatibilities[0]
assert.Equal(t, defaultK8sVersion, aws.StringValue(c.ClusterVersion))
assert.Equal(t, []string{"*"}, aws.StringValueSlice(c.PlatformVersions))
if aws.BoolValue(c.DefaultVersion) {
defaults++
assert.Equal(t, spec.AWSVersion(spec.DefaultVersion), aws.StringValue(v.AddonVersion))
}
}
assert.Equal(t, 1, defaults, "exactly one defaultVersion true")
}
// AWS observed (EKS-01): compatibilities holds only the requested version. The
// response for a version no add-on supports is not captured; this pins the
// shape #61 chose, an empty list with no error.
func TestDescribeAddonVersions_UnsupportedKubernetesVersionIsEmpty(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
out := describeVersions(t, svc, &eks.DescribeAddonVersionsInput{
AddonName: aws.String(csiAddon),
KubernetesVersion: aws.String("1.29"),
})
assert.Empty(t, out.Addons)
assert.Nil(t, out.NextToken)
}
func TestDescribeAddonVersions_FiltersByNameAndHidesFixturesUnlessNamed(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
for _, name := range []string{csiAddon, "argocd", albController} {
out := describeVersions(t, svc, &eks.DescribeAddonVersionsInput{AddonName: aws.String(name)})
require.Len(t, out.Addons, 1, name)
assert.Equal(t, name, aws.StringValue(out.Addons[0].AddonName))
}
assert.Empty(t, describeVersions(t, svc, &eks.DescribeAddonVersionsInput{AddonName: aws.String("vpc-cni")}).Addons)
}
// maxResults counts add-ons, not versions; a page boundary never splits an
// add-on's version list, and the last page carries no token.
func TestDescribeAddonVersions_PagesByAddon(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
all := describeVersions(t, svc, &eks.DescribeAddonVersionsInput{})
require.Greater(t, len(all.Addons), 1)
assert.Nil(t, all.NextToken)
var names []string
in := &eks.DescribeAddonVersionsInput{MaxResults: aws.Int64(1)}
for range len(all.Addons) {
out := describeVersions(t, svc, in)
require.Len(t, out.Addons, 1)
names = append(names, aws.StringValue(out.Addons[0].AddonName))
assert.Equal(t, versionStrings(all.Addons[len(names)-1]), versionStrings(out.Addons[0]))
in.NextToken = out.NextToken
}
assert.Nil(t, in.NextToken, "last page carries no token")
var want []string
for _, a := range all.Addons {
want = append(want, aws.StringValue(a.AddonName))
}
assert.Equal(t, want, names)
}
// Every add-on carries the descriptive fields the data source and console read.
func TestDescribeAddonVersions_CarriesAddonAndVersionMetadata(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
for _, a := range describeVersions(t, svc, &eks.DescribeAddonVersionsInput{}).Addons {
name := aws.StringValue(a.AddonName)
assert.NotEmpty(t, aws.StringValue(a.Type), name)
assert.NotEmpty(t, aws.StringValue(a.Owner), name)
assert.NotEmpty(t, aws.StringValue(a.Publisher), name)
assert.Nil(t, a.MarketplaceInformation, name)
for _, v := range a.AddonVersions {
assert.NotEmpty(t, v.Architecture, name)
assert.NotNil(t, v.RequiresConfiguration, name)
assert.NotNil(t, v.RequiresIamPermissions, name)
}
}
}
// types, publishers and owners are model filters; each keeps add-ons whose
// field matches one of the given values.
func TestDescribeAddonVersions_FiltersByTypePublisherOwner(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
first := describeVersions(t, svc, &eks.DescribeAddonVersionsInput{}).Addons[0]
cases := []struct {
name string
match func(*eks.DescribeAddonVersionsInput, []*string)
value *string
}{
{"types", func(in *eks.DescribeAddonVersionsInput, v []*string) { in.Types = v }, first.Type},
{"publishers", func(in *eks.DescribeAddonVersionsInput, v []*string) { in.Publishers = v }, first.Publisher},
{"owners", func(in *eks.DescribeAddonVersionsInput, v []*string) { in.Owners = v }, first.Owner},
}
for _, c := range cases {
hit := &eks.DescribeAddonVersionsInput{}
c.match(hit, aws.StringSlice([]string{"nobody", aws.StringValue(c.value)}))
out := describeVersions(t, svc, hit)
require.NotEmpty(t, out.Addons, c.name)
assert.Contains(t, addonNames(out), aws.StringValue(first.AddonName), c.name)
miss := &eks.DescribeAddonVersionsInput{}
c.match(miss, aws.StringSlice([]string{"nobody"}))
assert.Empty(t, describeVersions(t, svc, miss).Addons, c.name)
}
}
func versionStrings(a *eks.AddonInfo) []string {
out := make([]string, 0, len(a.AddonVersions))
for _, v := range a.AddonVersions {
out = append(out, aws.StringValue(v.AddonVersion))
}
return out
}
func addonNames(o *eks.DescribeAddonVersionsOutput) []string {
out := make([]string, 0, len(o.Addons))
for _, a := range o.Addons {
out = append(out, aws.StringValue(a.AddonName))
}
return out
}
@@ -1,220 +0,0 @@
package cluster
import (
"bytes"
"context"
"log/slog"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/eks"
"github.com/mulgadc/spinifex/spinifex/domains/eks/addon"
awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const (
csiAddon = "aws-ebs-csi-driver"
csiBare = "1.40.1"
csiAWSVersion = "v1.40.1-eksbuild.1"
csiWrongBuild = "v1.40.1-eksbuild.2"
versionParam = "addonVersion"
notBeforeAWSOK = "an AWS-format version must be accepted"
)
func TestDescribeAddonVersions_ReturnsAWSFormNewestFirst(t *testing.T) {
t.Parallel()
svc := setupTestService(t)
out, err := svc.DescribeAddonVersions(context.Background(), &eks.DescribeAddonVersionsInput{}, testAccountID)
require.NoError(t, err)
require.NotEmpty(t, out.Addons)
for _, a := range out.Addons {
spec, ok := addon.Lookup(aws.StringValue(a.AddonName))
require.True(t, ok)
require.Len(t, a.AddonVersions, len(spec.Versions))
for i, v := range a.AddonVersions {
assert.Equal(t, "v"+spec.Versions[i]+"-eksbuild.1", aws.StringValue(v.AddonVersion),
"%s version %d", spec.Name, i)
}
}
}
func TestCreateAddon_AcceptsBothFormsAndStoresTheBareForm(t *testing.T) {
t.Parallel()
for name, input := range map[string]string{"aws form": csiAWSVersion, "bare form": csiBare} {
t.Run(name, func(t *testing.T) {
t.Parallel()
svc, fake := setupAddonService(t)
out, err := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
AddonVersion: aws.String(input),
}, testAccountID)
require.NoError(t, err, notBeforeAWSOK)
assert.Equal(t, csiAWSVersion, aws.StringValue(out.Addon.AddonVersion), "response is in AWS form")
rec, err := addon.Get(t.Context(), acctKVForTest(t, svc), "c1", csiAddon)
require.NoError(t, err)
assert.Equal(t, csiBare, rec.AddonVersion, "the record keeps the upstream form")
require.Len(t, fake.installs, 1)
assert.Equal(t, csiBare, fake.installs[0].AddonVersion, "the bundle lookup sees the upstream form")
})
}
}
func TestCreateAddon_DefaultVersionResponseIsAWSForm(t *testing.T) {
t.Parallel()
svc, _ := setupAddonService(t)
out, err := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
}, testAccountID)
require.NoError(t, err)
assert.Equal(t, csiAWSVersion, aws.StringValue(out.Addon.AddonVersion))
}
func TestUpdateAddon_AcceptsBothFormsAndStoresTheBareForm(t *testing.T) {
t.Parallel()
for name, input := range map[string]string{"aws form": csiAWSVersion, "bare form": csiBare} {
t.Run(name, func(t *testing.T) {
t.Parallel()
svc, fake := setupAddonService(t)
_, err := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
}, testAccountID)
require.NoError(t, err)
_, err = svc.UpdateAddon(context.Background(), &eks.UpdateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
AddonVersion: aws.String(input),
}, testAccountID)
require.NoError(t, err, notBeforeAWSOK)
rec, err := addon.Get(t.Context(), acctKVForTest(t, svc), "c1", csiAddon)
require.NoError(t, err)
assert.Equal(t, csiBare, rec.AddonVersion)
require.Len(t, fake.installs, 2)
assert.Equal(t, csiBare, fake.installs[1].AddonVersion)
})
}
}
func TestDescribeAddon_ReturnsAWSFormForAnyStoredForm(t *testing.T) {
t.Parallel()
for name, stored := range map[string]string{
"record stored bare before the change": csiBare,
"record already in AWS form": csiAWSVersion,
} {
t.Run(name, func(t *testing.T) {
t.Parallel()
svc, _ := setupAddonService(t)
_, err := svc.addons().Create(context.Background(), acctKVForTest(t, svc), testAccountID, addon.Desired{
Cluster: "c1", Name: csiAddon, Version: stored,
})
require.NoError(t, err)
desc, err := svc.DescribeAddon(context.Background(), &eks.DescribeAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
}, testAccountID)
require.NoError(t, err)
assert.Equal(t, csiAWSVersion, aws.StringValue(desc.Addon.AddonVersion), "no double wrap")
list, err := svc.ListAddons(context.Background(), &eks.ListAddonsInput{ClusterName: aws.String("c1")}, testAccountID)
require.NoError(t, err)
assert.Equal(t, []string{csiAddon}, aws.StringValueSlice(list.Addons))
})
}
}
func TestUpdateAddon_ResponseAndRecordAfterBareRecord(t *testing.T) {
t.Parallel()
svc, _ := setupAddonService(t)
_, err := svc.addons().Create(context.Background(), acctKVForTest(t, svc), testAccountID, addon.Desired{
Cluster: "c1", Name: csiAddon, Version: csiBare,
})
require.NoError(t, err)
_, err = svc.UpdateAddon(context.Background(), &eks.UpdateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
ConfigurationValues: aws.String(`{"a":1}`),
}, testAccountID)
require.NoError(t, err)
desc, err := svc.DescribeAddon(context.Background(), &eks.DescribeAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
}, testAccountID)
require.NoError(t, err)
assert.Equal(t, csiAWSVersion, aws.StringValue(desc.Addon.AddonVersion))
}
func TestAddonVersionRefusal_NamesTheParameterAndListsSupportedValues(t *testing.T) {
t.Parallel()
svc, _ := setupAddonService(t)
_, err := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String("argocd"),
}, testAccountID)
require.NoError(t, err)
refusals := map[string]func() error{
"create": func() error {
_, e := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
AddonVersion: aws.String(csiWrongBuild),
}, testAccountID)
return e
},
"update": func() error {
_, e := svc.UpdateAddon(context.Background(), &eks.UpdateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String("argocd"),
AddonVersion: aws.String("v3.0.23-eksbuild.2"),
}, testAccountID)
return e
},
}
want := map[string]string{"create": csiAWSVersion, "update": "v3.0.23-eksbuild.1"}
for name, refuse := range refusals {
t.Run(name, func(t *testing.T) {
t.Parallel()
err := refuse()
require.Error(t, err)
code, message, ok := awserrors.ResolveErrorDetail(err)
require.True(t, ok)
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code, "this change leaves the error code alone")
assert.Contains(t, message, versionParam)
assert.Contains(t, message, want[name], "the supported AWS-form value is listed")
})
}
}
func TestAddonBareVersionInput_LogsItsUseAtInfo(t *testing.T) {
var buf bytes.Buffer
restore := slog.Default()
slog.SetDefault(slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelInfo})))
t.Cleanup(func() { slog.SetDefault(restore) })
svc, _ := setupAddonService(t)
_, err := svc.CreateAddon(context.Background(), &eks.CreateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
}, testAccountID)
require.NoError(t, err)
buf.Reset()
_, err = svc.UpdateAddon(context.Background(), &eks.UpdateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
AddonVersion: aws.String(csiBare),
}, testAccountID)
require.NoError(t, err)
logged := buf.String()
assert.Contains(t, logged, "level=INFO")
assert.Contains(t, logged, "bare")
assert.Contains(t, logged, csiAddon)
assert.Contains(t, logged, "c1")
buf.Reset()
_, err = svc.UpdateAddon(context.Background(), &eks.UpdateAddonInput{
ClusterName: aws.String("c1"), AddonName: aws.String(csiAddon),
AddonVersion: aws.String(csiAWSVersion),
}, testAccountID)
require.NoError(t, err, notBeforeAWSOK)
assert.NotContains(t, buf.String(), "bare", "the AWS form is the intended form and logs nothing")
}
+18 -120
View File
@@ -2,12 +2,8 @@ package cluster
import (
"context"
"encoding/base64"
"errors"
"fmt"
"log/slog"
"slices"
"strings"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/eks"
@@ -39,20 +35,15 @@ func (s *EKSServiceImpl) ListAddons(ctx context.Context, input *eks.ListAddonsIn
return &eks.ListAddonsOutput{Addons: aws.StringSlice(names)}, nil
}
// DescribeAddonVersions returns the add-on catalogue filtered by name, owner,
// publisher, type and the Kubernetes version its versions support, one page
// of add-ons at a time.
func (s *EKSServiceImpl) DescribeAddonVersions(_ context.Context, input *eks.DescribeAddonVersionsInput, _ string) (*eks.DescribeAddonVersionsOutput, error) {
if input == nil {
input = &eks.DescribeAddonVersionsInput{}
// DescribeAddonVersions returns the static add-on catalog, optionally filtered by name.
func (s *EKSServiceImpl) DescribeAddonVersions(ctx context.Context, input *eks.DescribeAddonVersionsInput, _ string) (*eks.DescribeAddonVersionsOutput, error) {
filter := ""
if input != nil {
filter = aws.StringValue(input.AddonName)
}
// Every catalogue version supports only the K3s line the node image ships.
if v := aws.StringValue(input.KubernetesVersion); v != "" && v != defaultK8sVersion {
return &eks.DescribeAddonVersionsOutput{Addons: []*eks.AddonInfo{}}, nil
}
filter := aws.StringValue(input.AddonName)
var matched []addon.Spec
for _, spec := range addon.Specs() {
specs := addon.Specs()
out := make([]*eks.AddonInfo, 0, len(specs))
for _, spec := range specs {
if filter != "" && spec.Name != filter {
continue
}
@@ -61,51 +52,9 @@ func (s *EKSServiceImpl) DescribeAddonVersions(_ context.Context, input *eks.Des
if spec.Hidden && filter == "" {
continue
}
if !matchesAny(input.Types, spec.Type) || !matchesAny(input.Owners, addonOwner) ||
!matchesAny(input.Publishers, addonPublisher) {
continue
}
matched = append(matched, spec)
}
page, next, err := pageAddons(matched, input.MaxResults, input.NextToken)
if err != nil {
return nil, err
}
out := make([]*eks.AddonInfo, 0, len(page))
for _, spec := range page {
out = append(out, addonSpecToAWS(spec))
}
return &eks.DescribeAddonVersionsOutput{Addons: out, NextToken: next}, nil
}
// matchesAny reports whether the filter is empty or lists value.
func matchesAny(filter []*string, value string) bool {
return len(filter) == 0 || slices.Contains(aws.StringValueSlice(filter), value)
}
// maxAddonPage bounds maxResults, the model's range being 1 to 100.
const maxAddonPage = 100
// pageAddons returns the add-ons after the token, at most maxResults of them.
// The token is the base64 name of the last add-on returned, opaque to clients.
func pageAddons(specs []addon.Spec, maxResults *int64, token *string) ([]addon.Spec, *string, error) {
if t := aws.StringValue(token); t != "" {
raw, err := base64.RawURLEncoding.DecodeString(t)
if err != nil {
return nil, nil, awserrors.Errorf(awserrors.ErrorEKSInvalidParameter, "nextToken is not valid")
}
after := string(raw)
specs = slices.DeleteFunc(slices.Clone(specs), func(sp addon.Spec) bool { return sp.Name <= after })
}
limit := len(specs)
if maxResults != nil {
limit = min(max(int(*maxResults), 1), maxAddonPage)
}
if limit >= len(specs) {
return specs, nil, nil
}
next := base64.RawURLEncoding.EncodeToString([]byte(specs[limit-1].Name))
return specs[:limit], &next, nil
return &eks.DescribeAddonVersionsOutput{Addons: out}, nil
}
// validateAddonServiceAccountRoleArn rejects a non-empty value that is not
@@ -140,12 +89,11 @@ func (s *EKSServiceImpl) CreateAddon(ctx context.Context, input *eks.CreateAddon
if !ok {
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
}
version := spec.DefaultVersion
if requested := aws.StringValue(input.AddonVersion); requested != "" {
var err error
if version, err = resolveAddonVersion(ctx, spec, requested, cluster); err != nil {
return nil, err
}
version := aws.StringValue(input.AddonVersion)
if version == "" {
version = spec.DefaultVersion
} else if !spec.SupportsVersion(version) {
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
}
acctKV, err := s.acctKVForNewDependant(ctx, accountID, cluster)
if err != nil {
@@ -249,13 +197,9 @@ func (s *EKSServiceImpl) UpdateAddon(ctx context.Context, input *eks.UpdateAddon
version := aws.StringValue(input.AddonVersion)
if version != "" {
spec, ok := addon.Lookup(addonName)
if !ok {
if !ok || !spec.SupportsVersion(version) {
return nil, errors.New(awserrors.ErrorInvalidParameterValue)
}
var err error
if version, err = resolveAddonVersion(ctx, spec, version, cluster); err != nil {
return nil, err
}
}
rec, err := s.addons().Update(ctx, acctKV, accountID, cluster, addonName, addon.Change{
Version: version,
@@ -302,7 +246,7 @@ func addonRecordToAWS(cluster string, rec *addon.Record) *eks.Addon {
out := &eks.Addon{
AddonArn: aws.String(rec.Arn),
AddonName: aws.String(rec.AddonName),
AddonVersion: aws.String(awsAddonVersion(rec)),
AddonVersion: aws.String(rec.AddonVersion),
ClusterName: aws.String(cluster),
Status: aws.String(string(rec.Status)),
CreatedAt: aws.Time(rec.CreatedAt),
@@ -325,36 +269,17 @@ func addonRecordToAWS(cluster string, rec *addon.Record) *eks.Addon {
return out
}
// The owner and publisher Spinifex reports for every catalogue add-on, as AWS
// does for the add-ons it publishes.
const (
addonOwner = "aws"
addonPublisher = "eks"
)
// addonSpecToAWS converts a catalog spec to the SDK AddonInfo shape. Every
// version supports the one cluster version the node image ships, and exactly
// one of them is that cluster version's default.
// addonSpecToAWS converts a catalog spec to the SDK AddonInfo shape.
func addonSpecToAWS(spec addon.Spec) *eks.AddonInfo {
versions := make([]*eks.AddonVersionInfo, 0, len(spec.Versions))
for _, v := range spec.AWSVersions() {
for _, v := range spec.Versions {
versions = append(versions, &eks.AddonVersionInfo{
AddonVersion: aws.String(v),
Architecture: aws.StringSlice([]string{"amd64"}),
RequiresConfiguration: aws.Bool(false),
RequiresIamPermissions: aws.Bool(spec.RequiresIRSA),
Compatibilities: []*eks.Compatibility{{
ClusterVersion: aws.String(defaultK8sVersion),
PlatformVersions: aws.StringSlice([]string{"*"}),
DefaultVersion: aws.Bool(v == spec.AWSVersion(spec.DefaultVersion)),
}},
})
}
return &eks.AddonInfo{
AddonName: aws.String(spec.Name),
Type: aws.String(spec.Type),
Owner: aws.String(addonOwner),
Publisher: aws.String(addonPublisher),
AddonVersions: versions,
}
}
@@ -383,30 +308,3 @@ func (s *EKSServiceImpl) addonBucket(ctx context.Context, accountID string) (jet
}
return GetOrCreateAccountBucket(ctx, js, accountID)
}
// resolveAddonVersion maps a client version, in AWS form or bare upstream form,
// to the upstream form the record and the bundle lookup use. The bare form is
// more lenient than AWS, so each use is logged.
func resolveAddonVersion(ctx context.Context, spec addon.Spec, requested, cluster string) (string, error) {
upstream, ok := spec.ParseVersion(requested)
if !ok {
return "", awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
"addonVersion %q is not supported for add-on %s; supported versions: %s",
requested, spec.Name, strings.Join(spec.AWSVersions(), ", "))
}
if requested == upstream {
slog.InfoContext(ctx, "EKS add-on version given in bare upstream form",
"addon", spec.Name, "cluster", cluster, "version", requested)
}
return upstream, nil
}
// awsAddonVersion returns a stored record's version in AWS form; an add-on the
// catalogue no longer lists keeps its stored string.
func awsAddonVersion(rec *addon.Record) string {
spec, ok := addon.Lookup(rec.AddonName)
if !ok {
return rec.AddonVersion
}
return spec.AWSVersion(rec.AddonVersion)
}
+4 -8
View File
@@ -98,9 +98,7 @@ func TestCreateAddon_UnknownVersionRejected(t *testing.T) {
ClusterName: aws.String("c1"), AddonName: aws.String(albController),
AddonVersion: aws.String("9.9.9-nope"),
}, testAccountID)
code, ok := awserrors.ResolveErrorCode(err)
require.True(t, ok)
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
require.EqualError(t, err, awserrors.ErrorInvalidParameterValue)
}
func TestCreateAddon_CreatesStagingAndDefaultsVersion(t *testing.T) {
@@ -115,7 +113,7 @@ func TestCreateAddon_CreatesStagingAndDefaultsVersion(t *testing.T) {
require.NotNil(t, out.Addon)
// Version defaults to catalog default; status starts CREATING (honest until
// VM-side delivery confirms ACTIVE).
assert.Equal(t, spec.AWSVersion(spec.DefaultVersion), aws.StringValue(out.Addon.AddonVersion))
assert.Equal(t, spec.DefaultVersion, aws.StringValue(out.Addon.AddonVersion))
assert.Equal(t, string(addon.StatusCreating), aws.StringValue(out.Addon.Status))
assert.Contains(t, aws.StringValue(out.Addon.AddonArn), ":addon/c1/"+albController)
@@ -216,7 +214,7 @@ func TestUpdateAddon_ChangesVersionAndReinstalls(t *testing.T) {
}, testAccountID)
require.NoError(t, err)
assert.Equal(t, `{"replicaCount":2}`, aws.StringValue(desc.Addon.ConfigurationValues))
assert.Equal(t, spec.AWSVersion(spec.DefaultVersion), aws.StringValue(desc.Addon.AddonVersion))
assert.Equal(t, spec.DefaultVersion, aws.StringValue(desc.Addon.AddonVersion))
assert.Equal(t, string(addon.StatusUpdating), aws.StringValue(desc.Addon.Status))
}
@@ -232,9 +230,7 @@ func TestUpdateAddon_UnknownVersionRejected(t *testing.T) {
ClusterName: aws.String("c1"), AddonName: aws.String("argocd"),
AddonVersion: aws.String("9.9.9-nope"),
}, testAccountID)
code, ok := awserrors.ResolveErrorCode(err)
require.True(t, ok)
assert.Equal(t, awserrors.ErrorInvalidParameterValue, code)
require.EqualError(t, err, awserrors.ErrorInvalidParameterValue)
}
func TestUpdateAddon_MissingIsNotFound(t *testing.T) {
+12 -12
View File
@@ -46,7 +46,7 @@ backend {{.Name}}
{{- if .Redirect}}
http-request redirect {{if .Redirect.Scheme}}scheme {{.Redirect.Scheme}}{{else}}location {{.Redirect.Location}}{{end}} code {{.Redirect.Code}}
{{- else if .FixedResponse}}
http-request return status {{.FixedResponse.StatusCode}}{{if .FixedResponse.ContentType}} content-type {{squote .FixedResponse.ContentType}}{{end}}{{if .FixedResponse.Body}} string {{squote .FixedResponse.Body}}{{end}}
http-request return status {{.FixedResponse.StatusCode}}{{if .FixedResponse.ContentType}} content-type "{{.FixedResponse.ContentType}}"{{end}}{{if .FixedResponse.Body}} string "{{.FixedResponse.Body}}"{{end}}
{{- else}}
balance roundrobin{{if .HealthCheckPath}}
option httpchk
@@ -65,14 +65,7 @@ backend {{.Name}}
{{end}}
`
var haproxyHTTPTmpl = template.Must(template.New("haproxy").Funcs(template.FuncMap{"squote": haproxySingleQuote}).Parse(haproxyConfigTemplate))
// haproxySingleQuote renders s as one literal HAProxy word. HAProxy expands
// environment variables inside double quotes but not single quotes, so tenant
// text is single-quoted with each embedded quote closed, escaped and reopened.
func haproxySingleQuote(s string) string {
return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'"
}
var haproxyHTTPTmpl = template.Must(template.New("haproxy").Parse(haproxyConfigTemplate))
// HAProxyConfig holds the full configuration for rendering the HAProxy template.
type HAProxyConfig struct {
@@ -573,10 +566,17 @@ func validHTTPStatusCode(s string) bool {
return true
}
// validFixedResponseBody rejects bytes that would end the single-line HAProxy
// directive the single-quoted body is rendered into.
// validFixedResponseBody rejects bytes that would terminate or inject into the HAProxy directive.
func validFixedResponseBody(s string) bool {
return s != "" && !strings.ContainsAny(s, "\n\r\x00")
if s == "" {
return false
}
for _, r := range s {
if r == '\n' || r == '\r' || r == '"' || r == '\\' || r == '#' {
return false
}
}
return true
}
// buildRedirect translates a RedirectAction into an HAProxy redirect directive.
+2 -54
View File
@@ -279,7 +279,7 @@ func TestGenerateHAProxyConfig_FixedResponseDefault(t *testing.T) {
// Synthetic default backend named off the listener ARN, returning the fixed response.
assert.Contains(t, config, "default_backend bkdefault_lst-616760baf6a3031c3")
assert.Contains(t, config, "backend bkdefault_lst-616760baf6a3031c3")
assert.Contains(t, config, `http-request return status 404 content-type 'text/plain' string 'no route'`)
assert.Contains(t, config, `http-request return status 404 content-type "text/plain" string "no route"`)
// Host-header rule still routes to the app backend.
assert.Contains(t, config, "use_backend bk_tg-app1 if")
assert.Contains(t, config, "10.42.0.9:8080")
@@ -309,59 +309,7 @@ func TestGenerateHAProxyConfig_FixedResponseRejectsUnsafeBody(t *testing.T) {
assert.Contains(t, config, "http-request return status 200")
assert.NotContains(t, config, "always_true")
assert.NotContains(t, config, `string 'evil`)
}
// TestGenerateHAProxyConfig_FixedResponseIsLiteral renders the content type and
// body in single quotes, where HAProxy performs no environment expansion, so a
// body referencing the lb-agent's environment is served verbatim.
func TestGenerateHAProxyConfig_FixedResponseIsLiteral(t *testing.T) {
t.Parallel()
lb := &LoadBalancerRecord{LoadBalancerID: "lb-env"}
listeners := []*ListenerRecord{
{
ListenerArn: "arn:lst-env",
Port: 80,
DefaultActions: []ListenerAction{
{Type: ActionTypeFixedResponse, FixedResponse: &FixedResponseAction{
StatusCode: "200",
ContentType: "text/plain",
MessageBody: `sk=${LB_SECRET_KEY} ak=$LB_ACCESS_KEY it's "q" \ #x`,
}},
},
},
}
config, _, err := GenerateHAProxyConfigWithCerts(lb, listeners, nil, nil, "0.0.0.0", nil)
require.NoError(t, err)
assert.Contains(t, config, `http-request return status 200 content-type 'text/plain' string 'sk=${LB_SECRET_KEY} ak=$LB_ACCESS_KEY it'\''s "q" \ #x'`+"\n")
assert.NotContains(t, config, `"${LB_SECRET_KEY}`)
}
// TestGenerateHAProxyConfig_FixedResponseKeepsStoredTab keeps a stored body with
// a tab, which is literal inside single quotes and rendered before API checks existed.
func TestGenerateHAProxyConfig_FixedResponseKeepsStoredTab(t *testing.T) {
t.Parallel()
lb := &LoadBalancerRecord{LoadBalancerID: "lb-tab"}
listeners := []*ListenerRecord{
{
ListenerArn: "arn:lst-tab",
Port: 80,
DefaultActions: []ListenerAction{
{Type: ActionTypeFixedResponse, FixedResponse: &FixedResponseAction{
StatusCode: "503",
ContentType: "text/html",
MessageBody: "<pre>\tdown</pre>",
}},
},
},
}
config, _, err := GenerateHAProxyConfigWithCerts(lb, listeners, nil, nil, "0.0.0.0", nil)
require.NoError(t, err)
assert.Contains(t, config, "http-request return status 503 content-type 'text/html' string '<pre>\tdown</pre>'\n")
assert.NotContains(t, config, `string "evil`)
}
// TestGenerateHAProxyConfig_RedirectSchemeDefault renders the canonical
+6 -46
View File
@@ -17,8 +17,6 @@ import (
"strings"
"sync"
"time"
"unicode"
"unicode/utf8"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
@@ -2703,9 +2701,8 @@ func listenerActionFromSDK(a *elbv2.Action) ListenerAction {
return action
}
// validateListenerAction enforces the per-type action contract. Redirect and
// fixed-response fields are validated here so invalid input fails at the API,
// not at render time.
// validateListenerAction enforces the per-type action contract. Redirect fields
// are validated here so invalid input fails at the API, not at render time.
func validateListenerAction(a ListenerAction) error {
if a.Type == ActionTypeRedirect {
if a.Redirect == nil {
@@ -2713,39 +2710,6 @@ func validateListenerAction(a ListenerAction) error {
}
return validateRedirectAction(a.Redirect)
}
if a.Type == ActionTypeFixedResponse {
return validateFixedResponseAction(a.FixedResponse)
}
return nil
}
// fixedResponseContentTypes are the only ContentType values ELBv2 accepts.
var fixedResponseContentTypes = map[string]bool{
"text/plain": true,
"text/css": true,
"text/html": true,
"application/javascript": true,
"application/json": true,
}
// maxFixedResponseBodyLen is ELBv2's MessageBody length limit in characters.
const maxFixedResponseBodyLen = 1024
// validateFixedResponseAction rejects a ContentType ELBv2 does not allow, or a
// MessageBody that is too long or holds control characters.
func validateFixedResponseAction(fr *FixedResponseAction) error {
if fr == nil {
return errors.New(awserrors.ErrorMissingParameter)
}
if fr.ContentType != "" && !fixedResponseContentTypes[fr.ContentType] {
return errors.New(awserrors.ErrorInvalidParameterValue)
}
if utf8.RuneCountInString(fr.MessageBody) > maxFixedResponseBodyLen {
return errors.New(awserrors.ErrorInvalidParameterValue)
}
if strings.ContainsFunc(fr.MessageBody, unicode.IsControl) {
return errors.New(awserrors.ErrorInvalidParameterValue)
}
return nil
}
@@ -3539,18 +3503,14 @@ func (s *ELBv2ServiceImpl) tgRecordToSDK(r *TargetGroupRecord, lbArns []string)
HealthCheckEnabled: aws.Bool(r.HealthCheck.Enabled),
HealthCheckProtocol: aws.String(r.HealthCheck.Protocol),
HealthCheckPort: aws.String(r.HealthCheck.Port),
HealthCheckPath: aws.String(r.HealthCheck.Path),
HealthCheckIntervalSeconds: aws.Int64(r.HealthCheck.IntervalSeconds),
HealthCheckTimeoutSeconds: aws.Int64(r.HealthCheck.TimeoutSeconds),
HealthyThresholdCount: aws.Int64(r.HealthCheck.HealthyThreshold),
UnhealthyThresholdCount: aws.Int64(r.HealthCheck.UnhealthyThreshold),
}
// AWS omits both keys on TCP target groups, which have no path or matcher.
if r.HealthCheck.Path != "" {
tg.HealthCheckPath = aws.String(r.HealthCheck.Path)
}
if r.HealthCheck.Matcher != "" {
tg.Matcher = &elbv2.Matcher{HttpCode: aws.String(r.HealthCheck.Matcher)}
Matcher: &elbv2.Matcher{
HttpCode: aws.String(r.HealthCheck.Matcher),
},
}
// Omitted for NLB (TCP/UDP/TLS) target groups, matching AWS, which returns
+2 -2
View File
@@ -591,8 +591,8 @@ func (s *ELBv2ServiceImpl) validateAndConvertRuleActions(ctx context.Context, in
return nil, err
}
case ActionTypeFixedResponse:
if err := validateFixedResponseAction(action.FixedResponse); err != nil {
return nil, err
if action.FixedResponse == nil {
return nil, errors.New(awserrors.ErrorMissingParameter)
}
default:
return nil, errors.New(awserrors.ErrorELBv2InvalidConfigurationRequest)
@@ -7,7 +7,6 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/elbv2"
awserrors "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -661,53 +660,3 @@ func sortByPriority(rs []*RuleRecord) []*RuleRecord {
}
return out
}
func TestValidateListenerAction_FixedResponse(t *testing.T) {
t.Parallel()
fixed := func(ct, body string) ListenerAction {
return ListenerAction{Type: ActionTypeFixedResponse, FixedResponse: &FixedResponseAction{
StatusCode: "200", ContentType: ct, MessageBody: body,
}}
}
for _, ct := range []string{"", "text/plain", "text/css", "text/html", "application/javascript", "application/json"} {
assert.NoError(t, validateListenerAction(fixed(ct, "ok")), ct)
}
assert.NoError(t, validateListenerAction(fixed("text/plain", "")))
assert.NoError(t, validateListenerAction(fixed("text/plain", "sk=${LB_SECRET_KEY} it's \"q\" #x")))
assert.NoError(t, validateListenerAction(fixed("text/plain", strings.Repeat("é", 1024))))
bad := []ListenerAction{
fixed("text/$LB_SECRET_KEY", "ok"),
fixed("text/xml", "ok"),
fixed("TEXT/PLAIN", "ok"),
fixed("text/plain", strings.Repeat("a", 1025)),
fixed("text/plain", "line\nacl x always_true"),
fixed("text/plain", "cr\r"),
fixed("text/plain", "nul\x00"),
}
for _, a := range bad {
assert.EqualError(t, validateListenerAction(a), awserrors.ErrorInvalidParameterValue, "%+v", *a.FixedResponse)
}
assert.EqualError(t, validateListenerAction(ListenerAction{Type: ActionTypeFixedResponse}), awserrors.ErrorMissingParameter)
}
func TestCreateRule_RejectsFixedResponseContentType(t *testing.T) {
t.Parallel()
env := newRuleTestEnv(t, "cr-badfixed")
_, err := env.svc.CreateRule(context.Background(), &elbv2.CreateRuleInput{
ListenerArn: aws.String(env.listenerArn),
Priority: aws.Int64(1),
Conditions: []*elbv2.RuleCondition{{Field: aws.String("path-pattern"), Values: aws.StringSlice([]string{"/x"})}},
Actions: []*elbv2.Action{{
Type: aws.String("fixed-response"),
FixedResponseConfig: &elbv2.FixedResponseActionConfig{
StatusCode: aws.String("200"),
ContentType: aws.String("text/$LB_SECRET_KEY"),
MessageBody: aws.String("ok"),
},
}},
}, testAccountID)
require.EqualError(t, err, awserrors.ErrorInvalidParameterValue)
}
+4 -5
View File
@@ -459,12 +459,11 @@ func TestCreateTargetGroup_TCPProtocol(t *testing.T) {
assert.Equal(t, int64(5432), *tg.Port)
// NLB health check defaults: TCP protocol, no path, no matcher.
assert.Equal(t, "TCP", *tg.HealthCheckProtocol)
assert.Nil(t, tg.HealthCheckPath)
assert.Nil(t, tg.Matcher)
assert.Equal(t, int64(30), *tg.HealthCheckIntervalSeconds)
assert.Empty(t, *tg.HealthCheckPath)
assert.Empty(t, *tg.Matcher.HttpCode)
assert.Equal(t, int64(10), *tg.HealthCheckTimeoutSeconds)
assert.Equal(t, int64(5), *tg.HealthyThresholdCount)
assert.Equal(t, int64(2), *tg.UnhealthyThresholdCount)
assert.Equal(t, int64(3), *tg.HealthyThresholdCount)
assert.Equal(t, int64(3), *tg.UnhealthyThresholdCount)
}
func TestCreateTargetGroup_NLBProtocols(t *testing.T) {
+2 -2
View File
@@ -99,8 +99,8 @@ const (
// Default health check values (NLB).
DefaultNLBHealthCheckInterval = 30
DefaultNLBHealthCheckTimeout = 10
DefaultNLBHealthyThreshold = 5
DefaultNLBUnhealthyThreshold = 2
DefaultNLBHealthyThreshold = 3
DefaultNLBUnhealthyThreshold = 3
DefaultNLBHealthCheckProtocol = ProtocolTCP
DefaultNLBHealthCheckPort = "traffic-port"
+1
View File
@@ -88,6 +88,7 @@ type Client interface {
CreateDHCPOptions(ctx context.Context, opts *nbdb.DHCPOptions) (string, error)
UpdateDHCPOptionsOptions(ctx context.Context, uuid string, options map[string]string) error
DeleteDHCPOptions(ctx context.Context, uuid string) error
FindDHCPOptionsByCIDR(ctx context.Context, cidr string) (*nbdb.DHCPOptions, error)
FindDHCPOptionsByExternalID(ctx context.Context, key, value string) (*nbdb.DHCPOptions, error)
ListDHCPOptions(ctx context.Context) ([]nbdb.DHCPOptions, error)
@@ -573,26 +573,31 @@ func conformDHCPOptions(t *testing.T, cli ovn.Client, ctx context.Context) {
require.NoError(t, err)
require.NotEmpty(t, uuid, "create must return the row UUID callers store")
byCIDR, err := cli.FindDHCPOptionsByCIDR(ctx, "10.0.1.0/24")
require.NoError(t, err)
assert.Equal(t, uuid, byCIDR.UUID)
assert.Equal(t, "10.0.1.1", byCIDR.Options["server_id"])
byExtID, err := cli.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-1")
require.NoError(t, err)
assert.Equal(t, uuid, byExtID.UUID)
assert.Equal(t, "10.0.1.0/24", byExtID.CIDR)
assert.Equal(t, "10.0.1.1", byExtID.Options["server_id"])
require.NoError(t, cli.UpdateDHCPOptionsOptions(ctx, uuid, map[string]string{
"server_id": "10.0.1.1", "lease_time": "7200",
}))
byExtID, err = cli.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-1")
byCIDR, err = cli.FindDHCPOptionsByCIDR(ctx, "10.0.1.0/24")
require.NoError(t, err)
assert.Equal(t, "7200", byExtID.Options["lease_time"], "update must replace the options map")
assert.Equal(t, "7200", byCIDR.Options["lease_time"], "update must replace the options map")
all, err := cli.ListDHCPOptions(ctx)
require.NoError(t, err)
assert.Len(t, all, 1)
_, err = cli.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-nope")
_, err = cli.FindDHCPOptionsByCIDR(ctx, "10.9.9.0/24")
require.Error(t, err)
assert.Contains(t, err.Error(), "subnet-nope")
assert.Contains(t, err.Error(), "10.9.9.0/24")
_, err = cli.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-nope")
assert.Error(t, err)
require.NoError(t, cli.DeleteDHCPOptions(ctx, uuid))
all, err = cli.ListDHCPOptions(ctx)
+14
View File
@@ -826,6 +826,20 @@ func (c *LiveClient) DeleteDHCPOptions(ctx context.Context, uuid string) error {
return nil
}
func (c *LiveClient) FindDHCPOptionsByCIDR(ctx context.Context, cidr string) (*nbdb.DHCPOptions, error) {
var options []nbdb.DHCPOptions
err := c.client.WhereCache(func(o *nbdb.DHCPOptions) bool {
return o.CIDR == cidr
}).List(ctx, &options)
if err != nil {
return nil, fmt.Errorf("find DHCP options by CIDR: %w", err)
}
if len(options) == 0 {
return nil, fmt.Errorf("DHCP options for CIDR %q not found", cidr)
}
return &options[0], nil
}
func (c *LiveClient) FindDHCPOptionsByExternalID(ctx context.Context, key, value string) (*nbdb.DHCPOptions, error) {
var options []nbdb.DHCPOptions
err := c.client.WhereCache(func(o *nbdb.DHCPOptions) bool {
+12
View File
@@ -443,6 +443,18 @@ func (m *Client) DeleteDHCPOptions(_ context.Context, uuid string) error {
return nil
}
func (m *Client) FindDHCPOptionsByCIDR(_ context.Context, cidr string) (*nbdb.DHCPOptions, error) {
m.Mu.Lock()
defer m.Mu.Unlock()
for _, opts := range m.DHCPOpts {
if opts.CIDR == cidr {
result := *opts
return &result, nil
}
}
return nil, fmt.Errorf("DHCP options for CIDR %q not found", cidr)
}
func (m *Client) FindDHCPOptionsByExternalID(_ context.Context, key, value string) (*nbdb.DHCPOptions, error) {
m.Mu.Lock()
defer m.Mu.Unlock()
+7 -34
View File
@@ -374,8 +374,7 @@ func (r *reconciler) applyPorts(ctx context.Context, intent IntentState, actual
continue
}
existing, err := r.ovn.GetLogicalSwitchPort(ctx, portName)
if err != nil {
if _, err := r.ovn.GetLogicalSwitchPort(ctx, portName); err != nil {
addrStr := spec.MAC.String() + " " + spec.PrivateIP.String()
lsp := &nbdb.LogicalSwitchPort{
Name: portName,
@@ -399,10 +398,6 @@ func (r *reconciler) applyPorts(ctx context.Context, intent IntentState, actual
continue
}
if !spec.SuppressDHCP {
r.convergePortDHCP(ctx, existing, spec.SubnetID, res)
}
currentPGs, err := r.ovn.ListPortGroupsForPort(ctx, portName)
if err != nil {
slog.Warn("reconcile/apply: list port groups for port failed", "port", portName, "err", err)
@@ -425,26 +420,6 @@ func (r *reconciler) applyPorts(ctx context.Context, intent IntentState, actual
r.pruneOrphanPorts(ctx, intent, res)
}
// convergePortDHCP points an existing LSP's dhcpv4_options at its subnet's
// current DHCP_Options row. The reference is weak, so deleting or recreating
// the row leaves the port without OVN DHCP until it is re-linked here.
func (r *reconciler) convergePortDHCP(ctx context.Context, lsp *nbdb.LogicalSwitchPort, subnetID string, res *passResult) {
dhcpOpts, err := r.ovn.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", subnetID)
if err != nil || dhcpOpts == nil {
return
}
if lsp.DHCPv4Options != nil && *lsp.DHCPv4Options == dhcpOpts.UUID {
return
}
lsp.DHCPv4Options = &dhcpOpts.UUID
if err := r.ovn.UpdateLogicalSwitchPort(ctx, lsp); err != nil {
slog.Warn("reconcile/apply: re-link port DHCP options failed", "port", lsp.Name, "subnet_id", subnetID, "err", err)
res.fail(classPort, lsp.Name, err)
return
}
slog.Info("reconcile/apply: re-linked port DHCP options", "port", lsp.Name, "subnet_id", subnetID)
}
// pruneOrphanPorts deletes guest LSPs whose spinifex:eni_id has no matching
// intent ENI, closing the create-only gap that leaks ports across instance
// terminate and host reinstall. DeletePort clears PG memberships then removes
@@ -897,8 +872,7 @@ func (r *reconciler) forgetIdlePorts(specs []policy.EIPSpec) {
// never re-asserted, so a vpcd/OVN rebuild dropped it and its guest-port hop never
// converged (inbound DNATs at the gateway but never reaches the guest; ICMP is
// answered by the gateway LR, so ping works while TCP hangs). A user EIP on the same
// private IP in the same VPC wins; the auto-assigned entry is skipped to avoid a
// duplicate rule.
// private IP wins; the auto-assigned entry is skipped to avoid a duplicate rule.
func (r *reconciler) floatingIPSpecs(intent IntentState) []policy.EIPSpec {
specs := make([]policy.EIPSpec, 0, len(intent.EIPs)+len(intent.Ports))
for _, spec := range intent.EIPs {
@@ -909,7 +883,7 @@ func (r *reconciler) floatingIPSpecs(intent IntentState) []policy.EIPSpec {
continue
}
logicalIP := p.PrivateIP.String()
if _, hasEIP := intent.EIPs[eipIntentKey(p.VPCID, logicalIP)]; hasEIP {
if _, hasEIP := intent.EIPs[logicalIP]; hasEIP {
continue
}
specs = append(specs, policy.EIPSpec{
@@ -1040,7 +1014,7 @@ func (r *reconciler) applyPublicInstanceEgress(ctx context.Context, intent Inten
}
}
for _, e := range intent.EIPs {
r.ensureEIPEgressExemption(ctx, intent, e.VPCID, subnetIDForIP(intent.Subnets, e.VPCID, e.LogicalIP), e.LogicalIP, res)
r.ensureEIPEgressExemption(ctx, intent, e.VPCID, subnetIDForIP(intent.Subnets, e.LogicalIP), e.LogicalIP, res)
}
}
@@ -1068,15 +1042,14 @@ func (r *reconciler) ensureEIPEgressExemption(ctx context.Context, intent Intent
}
}
// subnetIDForIP returns the SubnetID in vpcID whose CIDR contains ip, or "" if
// none match. VPC CIDRs overlap across tenants, so the CIDR alone is ambiguous.
func subnetIDForIP(subnets map[string]topology.SubnetSpec, vpcID, ip string) string {
// subnetIDForIP returns the SubnetID whose CIDR contains ip, or "" if none match.
func subnetIDForIP(subnets map[string]topology.SubnetSpec, ip string) string {
addr, err := netip.ParseAddr(ip)
if err != nil {
return ""
}
for _, s := range subnets {
if s.VPCID == vpcID && s.CIDR.IsValid() && s.CIDR.Contains(addr) {
if s.CIDR.IsValid() && s.CIDR.Contains(addr) {
return s.SubnetID
}
}
@@ -135,11 +135,11 @@ func TestApplyEIPs_HealthyPortDoesNotWaitForDeadOnes(t *testing.T) {
}
intent := IntentState{EIPs: map[string]policy.EIPSpec{
eipIntentKey("vpc-a", "10.0.0.1"): {VPCID: "vpc-a", ExternalIP: "192.0.2.1", LogicalIP: "10.0.0.1", PortName: live},
"10.0.0.1": {VPCID: "vpc-a", ExternalIP: "192.0.2.1", LogicalIP: "10.0.0.1", PortName: live},
}}
for i, name := range dead {
ip := "10.0.0." + string(rune('2'+i))
intent.EIPs[eipIntentKey("vpc-a", ip)] = policy.EIPSpec{VPCID: "vpc-a", ExternalIP: "192.0.2.9", LogicalIP: ip, PortName: name}
intent.EIPs[ip] = policy.EIPSpec{VPCID: "vpc-a", ExternalIP: "192.0.2.9", LogicalIP: ip, PortName: name}
}
res := &passResult{}
@@ -1,242 +0,0 @@
package reconcile
import (
"context"
"net/netip"
"slices"
"sync"
"testing"
ec2eip "github.com/mulgadc/spinifex/spinifex/domains/ec2/eip"
ec2vpc "github.com/mulgadc/spinifex/spinifex/domains/ec2/vpc"
"github.com/mulgadc/spinifex/spinifex/domains/network/external"
"github.com/mulgadc/spinifex/spinifex/domains/network/policy"
"github.com/mulgadc/spinifex/spinifex/domains/network/topology"
"github.com/nats-io/nats.go/jetstream"
)
// hostBindings stands in for the routed-mode host route and proxy-ARP table.
type hostBindings struct {
mu sync.Mutex
bound map[string]struct{}
}
func (h *hostBindings) hooks() policy.HostEIPBinder {
return policy.HostEIPBinder{
Bind: func(eip policy.EIPSpec, _ string) error {
h.mu.Lock()
defer h.mu.Unlock()
h.bound[eip.ExternalIP] = struct{}{}
return nil
},
Unbind: func(externalIP string) error {
h.mu.Lock()
defer h.mu.Unlock()
delete(h.bound, externalIP)
return nil
},
List: func() ([]string, error) {
h.mu.Lock()
defer h.mu.Unlock()
out := make([]string, 0, len(h.bound))
for ip := range h.bound {
out = append(out, ip)
}
slices.Sort(out)
return out, nil
},
}
}
// seedOverlappingTenants writes two accounts' default-style VPCs on the same
// CIDR, each with one ENI at 172.31.0.4. eni-a carries publicA on its stamp.
func seedOverlappingTenants(t *testing.T, js jetstream.JetStream, publicA string) {
t.Helper()
for _, tenant := range []struct{ acct, suffix, mac, public string }{
{"111111111111", "a", "02:00:00:00:0a:04", publicA},
{"222222222222", "b", "02:00:00:00:0b:04", ""},
} {
vpcID, subnetID, sgID, eniID := "vpc-"+tenant.suffix, "subnet-"+tenant.suffix, "sg-"+tenant.suffix, "eni-"+tenant.suffix
seedKV(t, js, ec2vpc.KVBucketVPCs, tenant.acct+"."+vpcID, ec2vpc.VPCRecord{
VpcId: vpcID, CidrBlock: "172.31.0.0/16", AZ: "us-east-1a", VNI: 100,
})
seedKV(t, js, ec2vpc.KVBucketSubnets, tenant.acct+"."+subnetID, ec2vpc.SubnetRecord{
SubnetId: subnetID, VpcId: vpcID, CidrBlock: "172.31.0.0/20",
})
seedKV(t, js, ec2vpc.KVBucketSecurityGroups, tenant.acct+"."+sgID, ec2vpc.SecurityGroupRecord{
GroupId: sgID, VpcId: vpcID,
})
seedKV(t, js, ec2vpc.KVBucketENIs, tenant.acct+"."+eniID, ec2vpc.ENIRecord{
NetworkInterfaceId: eniID, SubnetId: subnetID, VpcId: vpcID,
PrivateIpAddress: "172.31.0.4", MacAddress: tenant.mac,
SecurityGroupIds: []string{sgID}, PublicIpAddress: tenant.public,
})
}
}
func seedAssociatedEIP(t *testing.T, js jetstream.JetStream, acct, suffix, publicIP, mac string) {
t.Helper()
seedKV(t, js, ec2eip.KVBucketEIPs, acct+".eipalloc-"+suffix, ec2eip.EIPRecord{
AllocationId: "eipalloc-" + suffix, PublicIp: publicIP, State: "associated",
AssociationId: "eipassoc-" + suffix, ENIId: "eni-" + suffix,
PrivateIp: "172.31.0.4", MacAddress: mac, VpcId: "vpc-" + suffix,
})
}
// Two tenants' guests share a private IP on overlapping VPC CIDRs. Tenant B
// associating its own EIP must leave tenant A's public address, NAT row and
// host binding alone, whether A's address is a user EIP or auto-assigned.
func TestReconcile_OverlappingVPCsKeepEachTenantsPublicIP(t *testing.T) {
cases := []struct {
name string
publicA string
eipA bool
}{
{name: "eip beside eip", publicA: "203.0.113.10", eipA: true},
{name: "eip beside auto-assigned", publicA: "198.51.100.20"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
ctx := context.Background()
js := startKV(t)
seedOverlappingTenants(t, js, tc.publicA)
if tc.eipA {
seedAssociatedEIP(t, js, "111111111111", "a", tc.publicA, "02:00:00:00:0a:04")
}
r, m := newTestReconciler(t)
hosts := &hostBindings{bound: map[string]struct{}{}}
nat, err := policy.NewNATManager(m, policy.NATModeRouted, policy.WithHostEIPBinder(hosts.hooks()))
if err != nil {
t.Fatalf("NewNATManager: %v", err)
}
r.nat = nat
r.reloadIntent = func(ctx context.Context) (IntentState, error) {
return LoadIntentFromKV(ctx, js, "us-east-1a")
}
pass := func() IntentState {
t.Helper()
intent, err := r.reloadIntent(ctx)
if err != nil {
t.Fatalf("LoadIntentFromKV: %v", err)
}
if err := r.Reconcile(ctx, intent); err != nil {
t.Fatalf("Reconcile: %v", err)
}
if err := r.ReconcileHostEIPs(ctx); err != nil {
t.Fatalf("ReconcileHostEIPs: %v", err)
}
return intent
}
assertPublic := func(stage, externalIP, port string) {
t.Helper()
row := findNATByExternal(m, "dnat_and_snat", externalIP)
if row == nil {
t.Fatalf("%s: dnat_and_snat for %s missing", stage, externalIP)
}
if got := row.ExternalIDs["spinifex:logical_port"]; got != topology.Port(port) {
t.Errorf("%s: %s row owned by %q, want %q", stage, externalIP, got, topology.Port(port))
}
if _, ok := hosts.bound[externalIP]; !ok {
t.Errorf("%s: host binding for %s missing", stage, externalIP)
}
}
pass()
assertPublic("before tenant B associates", tc.publicA, "eni-a")
seedAssociatedEIP(t, js, "222222222222", "b", "203.0.113.20", "02:00:00:00:0b:04")
for _, stage := range []string{"after tenant B associates", "on the next pass"} {
intent := pass()
// eni-a's stamp alone would keep A's row alive, so check that
// intent itself holds every tenant's association.
wantEIPs := map[string]string{eipIntentKey("vpc-b", "172.31.0.4"): "203.0.113.20"}
if tc.eipA {
wantEIPs[eipIntentKey("vpc-a", "172.31.0.4")] = tc.publicA
}
if len(intent.EIPs) != len(wantEIPs) {
t.Errorf("%s: intent holds %d EIPs, want %d: %+v", stage, len(intent.EIPs), len(wantEIPs), intent.EIPs)
}
for key, externalIP := range wantEIPs {
if got := intent.EIPs[key].ExternalIP; got != externalIP {
t.Errorf("%s: intent EIP %s = %q, want %q", stage, key, got, externalIP)
}
}
assertPublic(stage, tc.publicA, "eni-a")
assertPublic(stage, "203.0.113.20", "eni-b")
}
})
}
}
// The drop-gate exemption must name the EIP's own subnet, not a subnet in
// another VPC whose CIDR happens to contain the same private IP.
func TestSubnetIDForIP_ScopedToVPC(t *testing.T) {
subnets := map[string]topology.SubnetSpec{
"subnet-a": {SubnetID: "subnet-a", VPCID: "vpc-a", CIDR: netip.MustParsePrefix("172.31.0.0/20")},
"subnet-b": {SubnetID: "subnet-b", VPCID: "vpc-b", CIDR: netip.MustParsePrefix("172.31.0.0/20")},
}
for range 20 {
if got := subnetIDForIP(subnets, "vpc-a", "172.31.0.4"); got != "subnet-a" {
t.Fatalf("vpc-a lookup = %q, want subnet-a", got)
}
if got := subnetIDForIP(subnets, "vpc-b", "172.31.0.4"); got != "subnet-b" {
t.Fatalf("vpc-b lookup = %q, want subnet-b", got)
}
}
if got := subnetIDForIP(subnets, "vpc-c", "172.31.0.4"); got != "" {
t.Errorf("lookup in a VPC with no subnets = %q, want empty", got)
}
}
// egressCall is one EnsureEIPInstanceEgress invocation.
type egressCall struct{ vpcID, subnetID, instanceIP string }
// recordingIGW records EnsureEIPInstanceEgress calls instead of writing OVN.
type recordingIGW struct {
external.IGWManager
calls []egressCall
}
func (g *recordingIGW) EnsureEIPInstanceEgress(_ context.Context, vpcID, subnetID, instanceIP string) error {
g.calls = append(g.calls, egressCall{vpcID, subnetID, instanceIP})
return nil
}
// An EIP's drop-gate exemption goes to its own VPC's subnet even when another
// tenant's gated subnet has the same CIDR.
func TestApplyPublicInstanceEgress_EIPExemptsOwnVPCSubnet(t *testing.T) {
cidr := netip.MustParsePrefix("172.31.0.0/20")
intent := IntentState{
Subnets: map[string]topology.SubnetSpec{
"subnet-a": {SubnetID: "subnet-a", VPCID: "vpc-a", CIDR: cidr},
"subnet-b": {SubnetID: "subnet-b", VPCID: "vpc-b", CIDR: cidr},
},
EIPs: map[string]policy.EIPSpec{
eipIntentKey("vpc-b", "172.31.0.4"): {VPCID: "vpc-b", ExternalIP: "203.0.113.20", LogicalIP: "172.31.0.4"},
},
DropGates: map[string]SubnetEgressIntent{},
}
for _, s := range intent.Subnets {
intent.DropGates[subnetEgressKey(s.SubnetID, netip.MustParsePrefix("0.0.0.0/0"))] = SubnetEgressIntent{
VPCID: s.VPCID, SubnetID: s.SubnetID, DestCIDR: netip.MustParsePrefix("0.0.0.0/0"),
}
}
for range 20 {
r, _ := newTestReconciler(t)
igw := new(recordingIGW)
r.igw = igw
res := new(passResult)
r.applyPublicInstanceEgress(context.Background(), intent, ActualState{}, res)
want := []egressCall{{"vpc-b", "subnet-b", "172.31.0.4"}}
if !slices.Equal(igw.calls, want) {
t.Fatalf("EnsureEIPInstanceEgress calls = %+v, want %+v", igw.calls, want)
}
if len(res.failures) != 0 {
t.Fatalf("pass failures = %+v, want none", res.failures)
}
}
}
@@ -27,7 +27,7 @@ func captureReconcileLogs(t *testing.T) *bytes.Buffer {
func idleEIPIntent(idle bool) IntentState {
intent := IntentState{EIPs: map[string]policy.EIPSpec{
eipIntentKey("vpc-a", "10.0.0.4"): {VPCID: "vpc-a", ExternalIP: "192.0.2.46", LogicalIP: "10.0.0.4", PortName: "port-eni-1"},
"10.0.0.4": {VPCID: "vpc-a", ExternalIP: "192.0.2.46", LogicalIP: "10.0.0.4", PortName: "port-eni-1"},
}}
if idle {
intent.IdlePorts = map[string]string{"port-eni-1": "i-stopped"}
@@ -115,53 +115,6 @@ func TestReconcile_PortSuppressDHCP(t *testing.T) {
}
}
// A deleted DHCP_Options row clears dhcpv4_options on every LSP that pointed at
// it. The next pass recreates the row under a new UUID and must re-link the
// existing ports to it, leaving a SuppressDHCP port unlinked.
func TestReconcile_ExistingPortRelinkedToRecreatedDHCPOptions(t *testing.T) {
rec, m := newTestReconciler(t)
ctx := context.Background()
intent := freshIntent(t)
mac, _ := net.ParseMAC("02:00:00:00:00:09")
intent.Ports["eni-static"] = topology.PortSpec{
PortID: "eni-static", SubnetID: "subnet-a", VPCID: "vpc-a",
PrivateIP: netip.MustParseAddr("10.0.1.11"), MAC: mac,
SGIDs: []string{"sg-a"}, SuppressDHCP: true,
}
if err := rec.Reconcile(ctx, intent); err != nil {
t.Fatalf("Reconcile #1: %v", err)
}
old, err := m.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-a")
if err != nil {
t.Fatalf("DHCP options after first pass: %v", err)
}
if err := m.DeleteDHCPOptions(ctx, old.UUID); err != nil {
t.Fatalf("DeleteDHCPOptions: %v", err)
}
portName := "port-" + intent.Ports["eni-a"].PortID
m.Ports[portName].DHCPv4Options = nil
if err := rec.Reconcile(ctx, intent); err != nil {
t.Fatalf("Reconcile #2: %v", err)
}
cur, err := m.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-a")
if err != nil {
t.Fatalf("DHCP options after second pass: %v", err)
}
if cur.UUID == old.UUID {
t.Fatalf("DHCP options row was not recreated")
}
if got := m.Ports[portName].DHCPv4Options; got == nil || *got != cur.UUID {
t.Errorf("existing LSP dhcpv4_options = %v, want %s", got, cur.UUID)
}
if got := m.Ports["port-eni-static"].DHCPv4Options; got != nil {
t.Errorf("SuppressDHCP LSP must stay unlinked, got %q", *got)
}
}
func TestReconcile_PortJoinsPortGroupAtomically(t *testing.T) {
rec, m := newTestReconciler(t)
ctx := context.Background()
@@ -1207,10 +1160,10 @@ func TestFloatingIPSpecs(t *testing.T) {
intent := IntentState{
EIPs: map[string]policy.EIPSpec{
// User EIP on 172.31.0.9.
eipIntentKey("vpc-a", "172.31.0.9"): {VPCID: "vpc-a", ExternalIP: "192.168.1.200", LogicalIP: "172.31.0.9", PortName: topology.Port("eni-eip")},
"172.31.0.9": {VPCID: "vpc-a", ExternalIP: "192.168.1.200", LogicalIP: "172.31.0.9", PortName: topology.Port("eni-eip")},
// User EIP that also has an auto-assigned public IP recorded on its
// port: the EIP must win and the auto-assigned entry be skipped.
eipIntentKey("vpc-a", "172.31.0.4"): {VPCID: "vpc-a", ExternalIP: "192.168.1.201", LogicalIP: "172.31.0.4", PortName: topology.Port("eni-dup")},
"172.31.0.4": {VPCID: "vpc-a", ExternalIP: "192.168.1.201", LogicalIP: "172.31.0.4", PortName: topology.Port("eni-dup")},
},
Ports: map[string]topology.PortSpec{
// Auto-assigned public IP with no user EIP -> must produce a spec.
@@ -1219,9 +1172,6 @@ func TestFloatingIPSpecs(t *testing.T) {
// Same private IP as the user EIP above -> deduped out.
"eni-dup": {PortID: "eni-dup", VPCID: "vpc-a", PrivateIP: netip.MustParseAddr("172.31.0.4"),
PublicIP: netip.MustParseAddr("192.168.1.117"), MAC: mac},
// Same private IP as vpc-a's user EIP but in another VPC -> kept.
"eni-other-vpc": {PortID: "eni-other-vpc", VPCID: "vpc-b", PrivateIP: netip.MustParseAddr("172.31.0.4"),
PublicIP: netip.MustParseAddr("192.168.1.118"), MAC: mac},
// No public IP -> skipped.
"eni-nopub": {PortID: "eni-nopub", VPCID: "vpc-b", PrivateIP: netip.MustParseAddr("172.31.0.6")},
},
@@ -1253,12 +1203,9 @@ func TestFloatingIPSpecs(t *testing.T) {
if _, ok := byExternal["192.168.1.117"]; ok {
t.Errorf("auto-assigned 192.168.1.117 should be deduped by the user EIP on 172.31.0.4")
}
if _, ok := byExternal["192.168.1.118"]; !ok {
t.Errorf("auto-assigned 192.168.1.118 in vpc-b must not be deduped by vpc-a's EIP on the same private IP")
}
// No public IP -> no spec for that port.
if len(specs) != 4 {
t.Errorf("want 4 specs (2 EIP + 2 auto), got %d: %+v", len(specs), specs)
if len(specs) != 3 {
t.Errorf("want 3 specs (2 EIP + 1 auto), got %d: %+v", len(specs), specs)
}
// A NAT gateway's address holds an snat, never a dnat_and_snat, so it must
@@ -1283,7 +1230,7 @@ func TestHostBindSpecs_CarriesNATGatewayAddresses(t *testing.T) {
intent := IntentState{
EIPs: map[string]policy.EIPSpec{
eipIntentKey("vpc-a", "172.31.0.9"): {VPCID: "vpc-a", ExternalIP: "192.168.1.200", LogicalIP: "172.31.0.9", PortName: topology.Port("eni-eip")},
"172.31.0.9": {VPCID: "vpc-a", ExternalIP: "192.168.1.200", LogicalIP: "172.31.0.9", PortName: topology.Port("eni-eip")},
},
NATGWs: map[string]policy.NATGWSpec{
// One gateway, two associated subnets: the same address arrives
@@ -1336,7 +1283,7 @@ func TestHostBindSpecs_DedupesAgainstAnExistingEIP(t *testing.T) {
specs := r.hostBindSpecs(IntentState{
EIPs: map[string]policy.EIPSpec{
eipIntentKey("vpc-a", "172.31.0.9"): {VPCID: "vpc-a", ExternalIP: "192.168.1.240", LogicalIP: "172.31.0.9",
"172.31.0.9": {VPCID: "vpc-a", ExternalIP: "192.168.1.240", LogicalIP: "172.31.0.9",
PortName: topology.Port("eni-eip"), MAC: mac.String()},
},
NATGWs: map[string]policy.NATGWSpec{
@@ -1484,7 +1431,7 @@ func TestReconcile_MarksIGWAttachedOnlyOnSuccess(t *testing.T) {
// A distributed-NAT gateway LRP is link-local and gates the probe off,
// so an EIP is what gives the datapath check a target.
intent := igwIntent(t)
intent.EIPs[eipIntentKey("vpc-a", "10.0.1.5")] = policy.EIPSpec{VPCID: "vpc-a", ExternalIP: "203.0.113.5", LogicalIP: "10.0.1.5"}
intent.EIPs["10.0.1.5"] = policy.EIPSpec{VPCID: "vpc-a", ExternalIP: "203.0.113.5", LogicalIP: "10.0.1.5"}
if err := rec.Reconcile(ctx, intent); err != nil {
t.Fatalf("Reconcile = %v, want nil", err)
}
@@ -1660,7 +1607,7 @@ func TestReconcile_FailedAddEIPReportsPassIncomplete(t *testing.T) {
rec.nat = &stubNAT{NATManager: rec.nat, addErr: errors.New("ovsdb unreachable")}
intent := freshIntent(t)
intent.EIPs[eipIntentKey("vpc-a", "10.0.1.10")] = policy.EIPSpec{
intent.EIPs["10.0.1.10"] = policy.EIPSpec{
VPCID: "vpc-a", ExternalIP: "192.168.1.10", LogicalIP: "10.0.1.10",
PortName: topology.Port("eni-a"),
}
+2 -8
View File
@@ -32,7 +32,7 @@ type IntentState struct {
Ports map[string]topology.PortSpec
SGs map[string]policy.SGSpec
IGWs map[string]external.IGWSpec // attached only
EIPs map[string]policy.EIPSpec // keyed by eipIntentKey; associated only
EIPs map[string]policy.EIPSpec // keyed by logicalIP; associated only
NATGWs map[string]policy.NATGWSpec
IGWRoutes map[string]SubnetEgressIntent // per (subnet, dest) IGW egress reroute
NATGWRoutes map[string]SubnetEgressIntent // per (subnet, dest) NATGW egress reroute
@@ -57,12 +57,6 @@ type SubnetEgressIntent struct {
DestCIDR netip.Prefix
}
// eipIntentKey is the EIPs map key. Private IPs repeat across VPCs (every
// default VPC is 172.31.0.0/16), so the VPC is part of the identity.
func eipIntentKey(vpcID, privateIP string) string {
return vpcID + "|" + privateIP
}
// subnetEgressKey is the IGWRoutes/NATGWRoutes map key.
func subnetEgressKey(subnetID string, prefix netip.Prefix) string {
return subnetID + "|" + prefix.String()
@@ -571,7 +565,7 @@ func loadEIPs(ctx context.Context, js jetstream.JetStream, localVPCs map[string]
}
// MAC drives distributed dnat_and_snat; empty falls back to centralised.
spec.MAC = rec.MacAddress
out[eipIntentKey(rec.VpcId, rec.PrivateIp)] = spec
out[rec.PrivateIp] = spec
}
return nil
}
@@ -183,10 +183,10 @@ func TestLoadIntentFromKV_EIPStateFilter(t *testing.T) {
t.Fatalf("LoadIntentFromKV: %v", err)
}
if _, ok := intent.EIPs[eipIntentKey("vpc-local", "10.0.1.5")]; !ok {
if _, ok := intent.EIPs["10.0.1.5"]; !ok {
t.Errorf("associated EIP missing from intent")
}
if _, ok := intent.EIPs[eipIntentKey("vpc-local", "10.0.1.6")]; ok {
if _, ok := intent.EIPs["10.0.1.6"]; ok {
t.Errorf("non-associated EIP leaked into intent")
}
}
@@ -60,7 +60,7 @@ func kitchenSinkIntent(t *testing.T) IntentState {
"vpc-a": {VPCID: "vpc-a", InternetGatewayID: "igw-a"},
},
EIPs: map[string]policy.EIPSpec{
eipIntentKey("vpc-a", "10.0.1.10"): {VPCID: "vpc-a", ExternalIP: "192.168.0.50", LogicalIP: "10.0.1.10",
"10.0.1.10": {VPCID: "vpc-a", ExternalIP: "192.168.0.50", LogicalIP: "10.0.1.10",
PortName: topology.Port("eni-pub"), MAC: "02:00:00:00:00:01"},
},
NATGWs: map[string]policy.NATGWSpec{
@@ -278,19 +278,10 @@ func (m *liveManager) DeleteSubnet(ctx context.Context, spec SubnetSpec) error {
if err := m.ovn.DeleteLogicalRouterPort(ctx, routerName, routerPortName); err != nil {
slog.Warn("topology: DeleteSubnet router port", "port", routerPortName, "err", err)
}
// Subnet CIDRs overlap across VPCs and accounts, so only the subnet_id
// external ID identifies this subnet's DHCP_Options rows.
if spec.SubnetID != "" {
dhcpOpts, err := m.ovn.ListDHCPOptions(ctx)
if err != nil {
slog.Warn("topology: DeleteSubnet list DHCP options", "subnet_id", spec.SubnetID, "err", err)
}
for _, opts := range dhcpOpts {
if opts.ExternalIDs["spinifex:subnet_id"] != spec.SubnetID {
continue
}
if err := m.ovn.DeleteDHCPOptions(ctx, opts.UUID); err != nil {
slog.Warn("topology: DeleteSubnet DHCP options", "uuid", opts.UUID, "subnet_id", spec.SubnetID, "err", err)
if spec.CIDR.IsValid() {
if dhcpOpts, err := m.ovn.FindDHCPOptionsByCIDR(ctx, spec.CIDR.String()); err == nil {
if err := m.ovn.DeleteDHCPOptions(ctx, dhcpOpts.UUID); err != nil {
slog.Warn("topology: DeleteSubnet DHCP options", "cidr", spec.CIDR.String(), "err", err)
}
}
}
@@ -1,86 +0,0 @@
package topology_test
import (
"context"
"net"
"net/netip"
"testing"
"time"
"github.com/mulgadc/spinifex/spinifex/domains/network/ovn"
"github.com/mulgadc/spinifex/spinifex/domains/network/ovn/ovntest"
"github.com/mulgadc/spinifex/spinifex/domains/network/topology"
)
// Two VPCs in different accounts commonly share a subnet CIDR (every default
// subnet is 172.31.0.0/20). Deleting one subnet, repeatedly, must never touch
// the other subnet's DHCP_Options row or the LSP reference to it.
func TestLiveManager_DeleteSubnet_SameCIDRLeavesOtherSubnetDHCP(t *testing.T) {
nb := ovntest.StartNB(t)
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
cli := ovn.NewLiveClient(nb.Endpoint)
if err := cli.Connect(ctx); err != nil {
t.Fatalf("LiveClient.Connect: %v", err)
}
t.Cleanup(cli.Close)
mgr := topology.NewLiveManager(cli)
vpcCIDR := netip.MustParsePrefix("172.31.0.0/16")
subCIDR := netip.MustParsePrefix("172.31.0.0/20")
victim := topology.SubnetSpec{SubnetID: "subnet-bbbb", VPCID: "vpc-bbbb", CIDR: subCIDR}
if err := mgr.EnsureVPC(ctx, topology.VPCSpec{VPCID: victim.VPCID, CIDR: vpcCIDR}); err != nil {
t.Fatalf("EnsureVPC victim: %v", err)
}
if err := mgr.EnsureSubnet(ctx, victim); err != nil {
t.Fatalf("EnsureSubnet victim: %v", err)
}
if err := mgr.EnsureSGPortGroup(ctx, "sg-bbbb"); err != nil {
t.Fatalf("EnsureSGPortGroup: %v", err)
}
mac, _ := net.ParseMAC("02:00:00:00:bb:01")
port := topology.PortSpec{
PortID: "eni-bbbb", SubnetID: victim.SubnetID, VPCID: victim.VPCID,
PrivateIP: netip.MustParseAddr("172.31.0.10"), MAC: mac, SGIDs: []string{"sg-bbbb"},
}
if err := mgr.EnsurePort(ctx, port); err != nil {
t.Fatalf("EnsurePort: %v", err)
}
victimRow, err := cli.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", victim.SubnetID)
if err != nil {
t.Fatalf("victim DHCP options: %v", err)
}
if err := mgr.EnsureVPC(ctx, topology.VPCSpec{VPCID: "vpc-aaaa", CIDR: vpcCIDR}); err != nil {
t.Fatalf("EnsureVPC other: %v", err)
}
for i := range 10 {
other := topology.SubnetSpec{SubnetID: "subnet-aaaa" + string(rune('0'+i)), VPCID: "vpc-aaaa", CIDR: subCIDR}
if err := mgr.EnsureSubnet(ctx, other); err != nil {
t.Fatalf("cycle %d EnsureSubnet: %v", i, err)
}
if err := mgr.DeleteSubnet(ctx, other); err != nil {
t.Fatalf("cycle %d DeleteSubnet: %v", i, err)
}
rows, err := cli.ListDHCPOptions(ctx)
if err != nil {
t.Fatalf("ListDHCPOptions: %v", err)
}
if len(rows) != 1 || rows[0].UUID != victimRow.UUID {
owners := make([]string, 0, len(rows))
for _, r := range rows {
owners = append(owners, r.ExternalIDs["spinifex:subnet_id"])
}
t.Fatalf("cycle %d: DHCP_Options rows owned by %v, want only %s", i, owners, victim.SubnetID)
}
lsp, err := cli.GetLogicalSwitchPort(ctx, topology.Port(port.PortID))
if err != nil {
t.Fatalf("GetLogicalSwitchPort: %v", err)
}
if lsp.DHCPv4Options == nil || *lsp.DHCPv4Options != victimRow.UUID {
t.Fatalf("cycle %d: victim LSP dhcpv4_options = %v, want %s", i, lsp.DHCPv4Options, victimRow.UUID)
}
}
}
@@ -473,7 +473,7 @@ func TestLiveManager_WithDNSServer(t *testing.T) {
ctx := context.Background()
_ = mgr.EnsureVPC(ctx, VPCSpec{VPCID: "vpc-dns", CIDR: netip.MustParsePrefix("10.6.0.0/16")})
_ = mgr.EnsureSubnet(ctx, SubnetSpec{SubnetID: "subnet-dns", VPCID: "vpc-dns", CIDR: netip.MustParsePrefix("10.6.1.0/24")})
opts, err := mockClient.FindDHCPOptionsByExternalID(ctx, "spinifex:subnet_id", "subnet-dns")
opts, err := mockClient.FindDHCPOptionsByCIDR(ctx, "10.6.1.0/24")
if err != nil {
t.Fatalf("DHCPOptions missing: %v", err)
}
+1 -12
View File
@@ -393,15 +393,6 @@ var (
// real ECS both use the restJson1 wire code "InvalidParameterException",
// so this gives EKS call sites their own name for the same registered code.
ErrorEKSInvalidParameter = ErrorECSInvalidParameter
// ErrorEKSInvalidRequest is the code the EKS access-entry operations declare
// in place of a parameter code.
ErrorEKSInvalidRequest = "InvalidRequestException"
// ErrorEKSBadRequest is the code the EKS tagging operations declare for a
// malformed request.
ErrorEKSBadRequest = "BadRequestException"
// ErrorEKSClient aliases ErrorECSClientException: DescribeCluster and
// DeleteCluster declare the same wire code "ClientException".
ErrorEKSClient = ErrorECSClientException
// ErrorECRInvalidParameter aliases ErrorECSInvalidParameter: every ECR
// operation declares the AWS JSON 1.1 wire code "InvalidParameterException".
ErrorECRInvalidParameter = ErrorECSInvalidParameter
@@ -1276,10 +1267,8 @@ var ErrorLookup = map[string]ErrorMessage{
ErrorResourceNotFound: {HTTPCode: 404, Message: "The specified resource was not found."},
ErrorEKSResourceInUse: {HTTPCode: 409, Message: "A cluster with this name already exists in this account and Region. Delete the existing cluster, or choose a different name."},
ErrorEKSResourceNotFound: {HTTPCode: 404, Message: "The specified cluster could not be found. You can view your available clusters with ListClusters. Amazon EKS clusters are Region specific."},
ErrorEKSInvalidRequest: {HTTPCode: 400, Message: "The request is invalid given the state of the cluster or access entry."},
ErrorEKSBadRequest: {HTTPCode: 400, Message: "A parameter of the request is not valid."},
ErrorECSClientException: {HTTPCode: 400, Message: "Unable to describe task definition."},
ErrorECSClusterNotFound: {HTTPCode: 400, Message: "Cluster not found."},
ErrorECSClusterNotFound: {HTTPCode: 400, Message: "The specified cluster was not found. You can view your available clusters with ListClusters. Amazon ECS clusters are Region specific."},
ErrorECSInvalidParameter: {HTTPCode: 400, Message: "The specified parameter is not valid. Review the available parameters for the API request."},
ErrorECSServiceNotFound: {HTTPCode: 400, Message: "The specified service was not found. You can view your available services with ListServices. Amazon ECS services are cluster specific and Region specific."},
ErrorRetryableError: {HTTPCode: 400, Message: "A request submitted by an AWS service on your behalf could not be completed. The requesting service might automatically retry the request."},
@@ -363,11 +363,11 @@ func endpointReferencer(procRoot string, disc discoveredNbdkit) (pid int, found
// cmdlineReferencesEndpoint checks a NUL-separated /proc/<pid>/cmdline for
// disc's endpoint. A socket mount's path appears verbatim in whichever
// argument named it (QEMU's nbd+unix:///?socket=<path> file= or its
// server.path=<path> -blockdev option). A TCP mount's 127.0.0.1:<port> pair
// may appear joined in one argument (a boot/EFI drive's nbd://host:port) or
// split across server.host=/server.port= within the same comma-joined
// -blockdev argument, so both shapes are checked.
// argument named it (an EFI -drive's file= or a -blockdev's server.path=). A
// TCP mount's 127.0.0.1:<port> pair may appear joined in one argument (an EFI
// pflash drive's nbd://host:port) or split across server.host=/server.port=
// within one -blockdev argument. The match is by substring, so the boot
// volume's file.server.path=/file.server.host=/file.server.port= also match.
func cmdlineReferencesEndpoint(cmdline []byte, disc discoveredNbdkit) bool {
args := bytes.Split(bytes.TrimRight(cmdline, "\x00"), []byte{0})
@@ -415,7 +415,7 @@ func TestReapOrphanedNbdkit_LiveReferencer_LeftRunningAndLogged(t *testing.T) {
// TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs proves
// endpointReferencer recognizes a TCP endpoint even when QEMU's -blockdev
// splits host and port into server.host=/server.port= within one argument,
// not just the joined host:port form a boot/EFI drive uses.
// not just the joined host:port form an EFI pflash drive uses.
func TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs(t *testing.T) {
pid := spawnLongRunningProcess(t)
procRoot := t.TempDir()
@@ -428,6 +428,46 @@ func TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs(t *test
assert.Never(t, func() bool { return !hostprocess.ProcessAlive(pid) }, 50*time.Millisecond, 5*time.Millisecond, "a TCP endpoint split across server.host=/server.port= must still be recognized as referenced")
}
// TestReapOrphanedNbdkit_BootBlockdevReferencer_LeftRunning pins the boot
// volume's -blockdev, whose server options carry a file. prefix: a live
// guest's root nbdkit named only that way must never be signalled.
func TestReapOrphanedNbdkit_BootBlockdevReferencer_LeftRunning(t *testing.T) {
socket := filepath.Join(t.TempDir(), "vol-boot-root.sock")
tests := []struct {
name string
arg string
disc func(pid int) discoveredNbdkit
}{
{
name: "unix socket",
arg: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=" + socket + ",file.export=,file.reconnect-delay=30",
disc: func(pid int) discoveredNbdkit {
return discoveredNbdkit{PID: pid, Volume: "vol-boot-unix1", Socket: socket}
},
},
{
name: "tcp",
arg: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=127.0.0.1,file.server.port=10897,file.export=,file.reconnect-delay=30",
disc: func(pid int) discoveredNbdkit {
return discoveredNbdkit{PID: pid, Volume: "vol-boot-tcp1", Port: 10897}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
pid := spawnLongRunningProcess(t)
procRoot := t.TempDir()
const referencerPID = 909092
writeReferencerFixture(t, procRoot, referencerPID, tt.arg)
logs := captureLogs(t)
assert.False(t, reapOrphanedNbdkit(procRoot, tt.disc(pid), reapReasonUnadoptable), "a boot volume's nbdkit must never be signalled")
assert.Never(t, func() bool { return !hostprocess.ProcessAlive(pid) }, 50*time.Millisecond, 5*time.Millisecond, "a boot volume's nbdkit must never be signalled")
assert.Contains(t, logs.String(), fmt.Sprintf("referencing_pid=%d", referencerPID))
})
}
}
// unreadableProcDir creates procRoot/<pid> with a cmdline file inside, then
// strips all permissions from the directory itself, so os.ReadFile on the
// cmdline path inside it fails with a permission error rather than
@@ -84,6 +84,10 @@ func buildDeviceMap(devices []qmp.BlockDevice) map[string]string {
name := dev.Device
pciIndex := extractPCIIndex(dev.QDev)
// A -blockdev boot node has no BlockBackend name; its node-name is "os".
if name == "" && pciIndex >= 0 && dev.Inserted != nil {
name = dev.Inserted.NodeName
}
if pciIndex < 0 {
// Hot-plugged device: extract name and port from QDev path.
@@ -127,6 +127,23 @@ func TestBuildDeviceMap(t *testing.T) {
},
wantLen: 4,
},
{
// A -blockdev boot node leaves the BlockBackend unnamed, so the
// entry is keyed by its node-name instead of "".
name: "blockdev boot device is named by its node-name",
devices: []qmp.BlockDevice{
{Device: "", Inserted: &qmp.BlockInserted{NodeName: "os"}, QDev: "/machine/peripheral-anon/device[0]/virtio-backend"},
{Device: "cloudinit", Inserted: &qmp.BlockInserted{NodeName: "#block123"}, QDev: "/machine/peripheral-anon/device[3]/virtio-backend"},
{Device: "", Inserted: &qmp.BlockInserted{NodeName: "nbd-vol-abc"}, QDev: "/machine/peripheral/vdisk-vol-abc/virtio-backend"},
},
want: map[string]string{
"os": "/dev/vda",
"cloudinit": "/dev/vdb",
"vdisk-vol-abc": "/dev/vdc",
},
absent: []string{"", "nbd-vol-abc"},
wantLen: 3,
},
{
// Boot devices returned out of order: lowest PCI index wins /dev/vda;
// hot-plugged devices sort after boot devices.
@@ -426,3 +443,15 @@ func TestLogGuestDeviceMap_LeavesBlockDeviceMappingsAlone(t *testing.T) {
assert.Equal(t, "/dev/sdf", aws.StringValue(v.Instance.BlockDeviceMappings[1].DeviceName),
"a data mapping must still name the device the caller asked for, not /dev/vdb")
}
// TestBuildDeviceMap_UnnamedBootDeviceWithoutMedium covers an unnamed
// boot-time BlockBackend that reports no inserted medium.
func TestBuildDeviceMap_UnnamedBootDeviceWithoutMedium(t *testing.T) {
devices := []qmp.BlockDevice{
{Device: "", Inserted: nil, QDev: "/machine/peripheral-anon/device[0]/virtio-backend"},
{Device: "cloudinit", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral-anon/device[3]/virtio-backend"},
}
var got map[string]string
require.NotPanics(t, func() { got = buildDeviceMap(devices) })
assert.Equal(t, "/dev/vdb", got["cloudinit"], "the unnamed device still holds its PCI slot")
}
+61 -11
View File
@@ -6,6 +6,8 @@ import (
"slices"
"strconv"
"strings"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/nbd"
)
// IsMicrovm reports whether machineType is the QEMU microvm machine, with or
@@ -162,12 +164,14 @@ type blkDeviceJSON struct {
NumQueues int `json:"num-queues,omitempty"`
BootIndex int `json:"bootindex,omitempty"`
Mapping []iothreadVQMapping `json:"iothread-vq-mapping"`
Werror string `json:"werror"`
Rerror string `json:"rerror"`
}
// BlkDeviceMapped returns a virtio-blk -device that spreads its virtqueues
// across a pool of poolSize IOThreads, round-robin. Falls back to BlkDevice's
// scalar iothread= form when poolSize <= 1, so the common small-guest case is
// byte-for-byte what it was before this existed.
// scalar iothread= form when poolSize <= 1, when there is one queue, or on an
// MMIO machine.
//
// Every queue is mapped exactly once and the scalar iothread= property is
// absent: QEMU treats the two as alternative assignments and rejects both.
@@ -197,6 +201,8 @@ func BlkDeviceMapped(machineType, drive, iothreadBase string, queues, bootIdx, p
NumQueues: queues,
BootIndex: bootIdx,
Mapping: mapping,
Werror: "stop",
Rerror: "stop",
})
if err != nil {
return Device{}, fmt.Errorf("marshal virtio-blk iothread-vq-mapping: %w", err)
@@ -204,9 +210,9 @@ func BlkDeviceMapped(machineType, drive, iothreadBase string, queues, bootIdx, p
return Device{Value: string(b)}, nil
}
// BlkDevice returns the appropriate QEMU virtio-blk device string for
// machineType. iothread and queues are omitted when empty/zero. bootIdx is
// only emitted for PCI machines (MMIO has no bootindex concept).
// BlkDevice returns the virtio-blk device string for machineType, omitting an
// empty iothread or zero queues; bootIdx is PCI-only. It sets werror/rerror=stop
// (see VolumeBlkDeviceArgs), as a -blockdev node has no BlockBackend to hold it.
func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Device {
var b strings.Builder
if IsMMIO(machineType) {
@@ -224,6 +230,7 @@ func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Dev
if !IsMMIO(machineType) && bootIdx > 0 {
fmt.Fprintf(&b, ",bootindex=%d", bootIdx)
}
b.WriteString(",werror=stop,rerror=stop")
return Device{Value: b.String()}
}
@@ -282,9 +289,9 @@ func HotplugEBSBus(port int) string {
// (rendered as a joined command-line string) so the two block-graph shapes
// cannot diverge.
//
// werror/rerror mirror the boot drive's on-error policy (see Drive.Werror):
// a data volume has no -drive-backed BlockBackend to carry it, so the qdev
// device itself sets werror=stop,rerror=stop.
// werror=stop,rerror=stop pauses the VM and holds a failed request, so a
// transient backend loss is resumed with cont instead of reaching the guest as
// EIO. A -blockdev node has no BlockBackend to carry it, so the device sets it.
func VolumeBlkDeviceArgs(volumeID, nodeName, iothreadID, bus string) []string {
return []string{
fmt.Sprintf("id=%s", VolumeDeviceID(volumeID)),
@@ -383,11 +390,54 @@ func (o NBDServerOpts) CommandLineArgs() []string {
// monitor-owned node that blockdev-del can later remove (unlike -drive,
// which only creates a BlockBackend with an auto-generated node-name).
func VolumeBlockdev(nodeName string, server NBDServerOpts) Blockdev {
opts := append([]string{"driver=nbd", fmt.Sprintf("node-name=%s", nodeName)}, server.CommandLineArgs()...)
opts = append(opts, "export=", fmt.Sprintf("reconnect-delay=%d", NBDReconnectDelaySeconds))
opts := append([]string{"driver=nbd", fmt.Sprintf("node-name=%s", nodeName)}, nbdNodeOpts("", server)...)
return Blockdev{Value: strings.Join(opts, ",")}
}
// nbdNodeOpts returns an nbd node's server, export and reconnect-delay options,
// each prefixed with prefix: "" on the nbd node itself, "file." under raw.
func nbdNodeOpts(prefix string, server NBDServerOpts) []string {
var opts []string
for _, arg := range server.CommandLineArgs() {
opts = append(opts, prefix+arg)
}
return append(opts, prefix+"export=", fmt.Sprintf("%sreconnect-delay=%d", prefix, NBDReconnectDelaySeconds))
}
// bootNodeName is the boot volume's block-node name, which its -device names
// with drive=.
const bootNodeName = "os"
// nbdServerOpts parses an NBD URI into the server options a blockdev takes.
func nbdServerOpts(uri string) (NBDServerOpts, error) {
serverType, socketPath, host, port, err := nbd.ParseNBDURI(uri)
if err != nil {
return NBDServerOpts{}, err
}
return NBDServerOpts{Type: serverType, Path: socketPath, Host: host, Port: port}, nil
}
// bootNBDBlockdev renders the boot volume's -blockdev for an NBD export: a raw
// node over an nbd node, the graph -drive format=raw,cache=none built, with the
// same O_DIRECT setting and reconnect delay.
func bootNBDBlockdev(nodeName string, server NBDServerOpts) Blockdev {
opts := append([]string{"driver=raw", fmt.Sprintf("node-name=%s", nodeName), "cache.direct=on", "file.driver=nbd"}, nbdNodeOpts("file.", server)...)
return Blockdev{Value: strings.Join(opts, ",")}
}
// exportBlockdev renders v's -blockdev as nodeName: a host_device node for a
// block export, otherwise the NBD node nbdBlockdev builds from its URI.
func exportBlockdev(v EBSRequest, kind volumeExportKind, nodeName string, nbdBlockdev func(string, NBDServerOpts) Blockdev) (Blockdev, error) {
if kind == volumeExportBlock {
return VolumeHostDeviceBlockdev(nodeName, HostDeviceOpts{Path: v.BlockDevice, SizeBytes: v.BlockSizeBytes}), nil
}
server, err := nbdServerOpts(v.NBDURI)
if err != nil {
return Blockdev{}, fmt.Errorf("parse NBDURI for volume %s: %w", v.Name, err)
}
return nbdBlockdev(nodeName, server), nil
}
// HostDeviceOpts is a host block device export. QEMU opens Path through the
// host_device protocol driver with O_DIRECT and native AIO, and a raw format
// node over it exposes only the first SizeBytes to the guest.
@@ -413,7 +463,7 @@ func (o HostDeviceOpts) QMPArgs(nodeName string) map[string]any {
}
// VolumeHostDeviceBlockdev renders the -blockdev equivalent of QMPArgs for a
// data volume backed by a host block device.
// volume backed by a host block device.
func VolumeHostDeviceBlockdev(nodeName string, dev HostDeviceOpts) Blockdev {
return Blockdev{Value: fmt.Sprintf(
"driver=raw,node-name=%s,size=%d,file.driver=host_device,file.filename=%s,file.aio=native,file.cache.direct=on",
+5 -5
View File
@@ -130,28 +130,28 @@ func TestNICQueues_ClampedWhenEnabled(t *testing.T) {
func TestBlkDevice_PCI(t *testing.T) {
d := BlkDevice("q35", "os", "ioth-os", 4, 1)
assert.Equal(t, "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=4,bootindex=1", d.Value)
assert.Equal(t, "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=4,bootindex=1,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_PCI_NoBootIdx(t *testing.T) {
d := BlkDevice("q35", "data", "ioth-data", 2, 0)
assert.Equal(t, "virtio-blk-pci,drive=data,iothread=ioth-data,num-queues=2", d.Value)
assert.Equal(t, "virtio-blk-pci,drive=data,iothread=ioth-data,num-queues=2,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_PCI_NoIOThread(t *testing.T) {
d := BlkDevice("q35", "data", "", 0, 0)
assert.Equal(t, "virtio-blk-pci,drive=data", d.Value)
assert.Equal(t, "virtio-blk-pci,drive=data,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_MMIO(t *testing.T) {
d := BlkDevice("microvm", "os", "ioth-os", 4, 1)
// MMIO has no bootindex
assert.Equal(t, "virtio-blk-device,drive=os,iothread=ioth-os,num-queues=4", d.Value)
assert.Equal(t, "virtio-blk-device,drive=os,iothread=ioth-os,num-queues=4,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_MMIO_NoIOThread(t *testing.T) {
d := BlkDevice("microvm,x-option-roms=off", "data", "", 0, 0)
assert.Equal(t, "virtio-blk-device,drive=data", d.Value)
assert.Equal(t, "virtio-blk-device,drive=data,werror=stop,rerror=stop", d.Value)
}
func TestRngDevice_PCI(t *testing.T) {
+27 -42
View File
@@ -315,7 +315,7 @@ func (m *Manager) startQEMU(instance *VM, appendConsole bool) error {
m.initENIRequests(instance)
instance.EBSRequests.Mu.Lock()
driveCfg, err := buildDrives(instance.EBSRequests.Requests, spec.VCPUs, instance.Config.MachineType)
driveCfg, err := buildDiskBootDrives(instance.EBSRequests.Requests, spec.VCPUs, instance.Config.MachineType)
instance.EBSRequests.Mu.Unlock()
if err != nil {
return err
@@ -1398,16 +1398,24 @@ func directBootLaunchConfig(base Config, requests []EBSRequest, cpuCount int) (C
return cfg, nil
}
// buildDiskBootDrives is buildDrives for a guest that boots from a volume: a
// request list with no boot volume is refused, as QEMU would start with
// nothing to boot from and the guest would never come up.
func buildDiskBootDrives(requests []EBSRequest, cpuCount int, machineType string) (driveConfig, error) {
if !slices.ContainsFunc(requests, func(r EBSRequest) bool { return r.Boot }) {
return driveConfig{}, errors.New("no boot volume among the instance's EBS requests")
}
return buildDrives(requests, cpuCount, machineType)
}
// buildDrives converts EBS requests into QEMU drive/iothread/device/blockdev
// configs. Returns an error if any volume has no valid export (see
// classifyVolumeExport), or if the EBS hot-plug port pool is exhausted while
// allocating one for a data volume.
//
// Boot volumes keep the existing anonymous if=none -drive shape exactly as
// before — they are not detachable (see ErrVolumeNotDetachable), so the
// anonymous-node problem this function otherwise fixes does not apply, and
// changing the boot drive risks boot order. EFI volumes emit pflash unit=1;
// the readonly CODE blob (unit=0) is added by Config.Execute.
// The boot volume is a -blockdev node named "os" under a bootindex=1 device
// that carries werror/rerror; it is never detached, so it keeps fixed names.
// EFI volumes emit pflash unit=1; Config.Execute adds the CODE blob (unit=0).
//
// Every other (data) volume is given the same named node-name/device-id/
// iothread-id shape AttachVolume's hot-plug path uses, via a -blockdev entry
@@ -1432,41 +1440,29 @@ func buildDrives(requests []EBSRequest, cpuCount int, machineType string) (drive
}
switch {
case kind == volumeExportBlock && (v.Boot || v.EFI):
// Boot and EFI keep the legacy -drive shape, which has no
// host_device form here; only data volumes take a block export.
return driveConfig{}, fmt.Errorf("volume %s: block device export is only supported for data volumes", v.Name)
case kind == volumeExportBlock && v.EFI:
// The VARS store is a pflash -drive, which has no host_device form.
return driveConfig{}, fmt.Errorf("volume %s: block device export is not supported for an EFI volume", v.Name)
case v.Boot:
drive := Drive{
File: v.NBDURI,
Format: "raw",
If: "none",
Media: "disk",
ID: "os",
Cache: "none",
// Pause the VM and hold the failed request rather than
// delivering EIO to the guest, which aborts its journal. A
// transient backend loss is then recovered with cont.
Werror: "stop",
Rerror: "stop",
ReconnectDelay: NBDReconnectDelaySeconds,
blockdev, err := exportBlockdev(*v, kind, bootNodeName, bootNBDBlockdev)
if err != nil {
return driveConfig{}, err
}
// The boot disk's virtqueues are spread across a pool rather than
// all served by one host thread. A pool of 1 emits exactly the
// command line this used to, which is every guest below 4 vCPUs.
// all served by one host thread. A pool of 1 keeps the scalar
// iothread= form, which is every guest below 4 vCPUs.
pool := BlkIOThreadPoolSize(cpuCount)
for i := range pool {
cfg.IOThreads = append(cfg.IOThreads, IOThread{ID: BlkIOThreadID("ioth-os", i, pool)})
}
dev, err := BlkDeviceMapped(machineType, drive.ID, "ioth-os", cpuCount, 1, pool)
dev, err := BlkDeviceMapped(machineType, bootNodeName, "ioth-os", cpuCount, 1, pool)
if err != nil {
return driveConfig{}, fmt.Errorf("boot volume %s: %w", v.Name, err)
}
cfg.Blockdevs = append(cfg.Blockdevs, blockdev)
cfg.Devices = append(cfg.Devices, dev)
cfg.Drives = append(cfg.Drives, drive)
case v.EFI:
cfg.Drives = append(cfg.Drives, Drive{
@@ -1495,20 +1491,9 @@ func buildDrives(requests []EBSRequest, cpuCount int, machineType string) (drive
nodeName := VolumeNodeName(v.Name)
iothreadID := VolumeIOThreadID(v.Name)
var blockdev Blockdev
if kind == volumeExportBlock {
blockdev = VolumeHostDeviceBlockdev(nodeName, HostDeviceOpts{Path: v.BlockDevice, SizeBytes: v.BlockSizeBytes})
} else {
serverType, socketPath, nbdHost, nbdPort, err := nbd.ParseNBDURI(v.NBDURI)
if err != nil {
return driveConfig{}, fmt.Errorf("parse NBDURI for volume %s: %w", v.Name, err)
}
blockdev = VolumeBlockdev(nodeName, NBDServerOpts{
Type: serverType,
Path: socketPath,
Host: nbdHost,
Port: nbdPort,
})
blockdev, err := exportBlockdev(*v, kind, nodeName, VolumeBlockdev)
if err != nil {
return driveConfig{}, err
}
cfg.IOThreads = append(cfg.IOThreads, IOThread{ID: iothreadID})
+142 -18
View File
@@ -132,12 +132,12 @@ func TestBuildDrives(t *testing.T) {
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
},
cpuCount: 2,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
},
wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
},
},
{
@@ -148,12 +148,12 @@ func TestBuildDrives(t *testing.T) {
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
},
cpuCount: 4,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
},
wantIOThreads: []IOThread{{ID: "ioth-os-0"}, {ID: "ioth-os-1"}},
wantDevices: []Device{
{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}]}`},
{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}],"werror":"stop","rerror":"stop"}`},
},
},
{
@@ -249,7 +249,7 @@ func TestBuildDrives(t *testing.T) {
wantErr: "no free EBS hot-plug port for volume vol-overflow",
},
{
name: "mixed boot + EFI + data: boot and EFI keep the legacy -drive shape",
name: "mixed boot + EFI + data: boot is a -blockdev node and EFI keeps the pflash -drive",
requests: []EBSRequest{
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
{Name: "vol-efi", NBDURI: "nbd:unix:/tmp/efi.sock", EFI: true},
@@ -260,7 +260,6 @@ func TestBuildDrives(t *testing.T) {
// unaffected by the pool either way — they keep one IOThread each.
cpuCount: 2,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
{File: "nbd:unix:/tmp/efi.sock", Format: "raw", If: "pflash", Unit: 1},
},
wantIOThreads: []IOThread{
@@ -268,10 +267,11 @@ func TestBuildDrives(t *testing.T) {
{ID: "ioth-vol-data-a"},
},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
{Value: "driver=nbd,node-name=nbd-vol-data-a,server.type=unix,server.path=/tmp/data-a.sock,export=,reconnect-delay=30"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
{Value: "virtio-blk-pci,id=vdisk-vol-data-a,drive=nbd-vol-data-a,iothread=ioth-vol-data-a,serial=voldataa,bus=hotplug-ebs3,werror=stop,rerror=stop"},
},
wantHotplugPorts: []int{0, 0, 3},
@@ -348,31 +348,72 @@ func TestBuildDrives(t *testing.T) {
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
{Name: "vol-blk", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20},
},
cpuCount: 2,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
},
cpuCount: 2,
wantIOThreads: []IOThread{{ID: "ioth-os"}, {ID: "ioth-vol-blk"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
{Value: "driver=raw,node-name=nbd-vol-blk,size=1048576,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
{Value: "virtio-blk-pci,id=vdisk-vol-blk,drive=nbd-vol-blk,iothread=ioth-vol-blk,serial=volblk,bus=hotplug-ebs1,werror=stop,rerror=stop"},
},
wantHotplugPorts: []int{0, 1},
},
{
name: "block boot volume is rejected",
requests: []EBSRequest{{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20, Boot: true}},
name: "block boot volume is a raw node over host_device named os under the bootindex device",
requests: []EBSRequest{
{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 8 << 30, Boot: true},
},
cpuCount: 2,
wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
},
wantHotplugPorts: []int{0},
},
{
name: "block boot volume with four vCPUs keeps the mapped device and error policy",
requests: []EBSRequest{
{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 8 << 30, Boot: true},
},
cpuCount: 4,
wantIOThreads: []IOThread{{ID: "ioth-os-0"}, {ID: "ioth-os-1"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
},
wantDevices: []Device{
{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}],"werror":"stop","rerror":"stop"}`},
},
},
{
name: "NBD boot volume over TCP renders inet server options under file.",
requests: []EBSRequest{
{Name: "vol-boot", NBDURI: "nbd://127.0.0.1:44801", Boot: true},
},
cpuCount: 2,
wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=127.0.0.1,file.server.port=44801,file.export=,file.reconnect-delay=30"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
},
},
{
name: "unparseable NBD boot URI is rejected",
requests: []EBSRequest{{Name: "vol-boot", NBDURI: "http://example/boot", Boot: true}},
cpuCount: 2,
wantErr: "volume vol-boot: block device export is only supported for data volumes",
wantErr: "parse NBDURI for volume vol-boot",
},
{
name: "block EFI volume is rejected",
requests: []EBSRequest{{Name: "vol-efi", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20, EFI: true}},
cpuCount: 2,
wantErr: "volume vol-efi: block device export is only supported for data volumes",
wantErr: "volume vol-efi: block device export is not supported for an EFI volume",
},
{
name: "both NBDURI and BlockDevice set is rejected",
@@ -1109,6 +1150,7 @@ func TestStartQEMU_DiskBoot_MgmtTapCreated(t *testing.T) {
InstanceType: "t3.nano",
MgmtMAC: "02:aa:bb:cc:dd:ee",
}
instance.EBSRequests.Requests = []EBSRequest{{Name: "vol-root", NBDURI: "nbd:unix:/tmp/root.sock", Boot: true}}
err := m.startQEMU(instance, false)
@@ -2168,6 +2210,88 @@ func TestStartQEMU_DiskBoot_DataVolumeStaysOnThePCIPath(t *testing.T) {
assert.Equal(t, 1, instance.EBSRequests.Requests[1].HotplugPort)
}
// TestStartQEMU_DiskBoot_BootVolumeIsABlockdevNode launches a disk-boot guest
// from each export kind and checks the boot volume reaches QEMU as the "os"
// -blockdev node behind the bootindex=1 device, with no -drive left for it.
func TestStartQEMU_DiskBoot_BootVolumeIsABlockdevNode(t *testing.T) {
tests := []struct {
name string
req func(t *testing.T) EBSRequest
wantBlockdev func(req EBSRequest) string
}{
{
name: "nbd export",
req: func(t *testing.T) EBSRequest {
return EBSRequest{Name: "vol-root", NBDURI: nbdListener(t), Boot: true}
},
wantBlockdev: func(req EBSRequest) string {
hostPort := strings.TrimPrefix(req.NBDURI, "nbd://")
host, port, _ := strings.Cut(hostPort, ":")
return "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=" + host +
",file.server.port=" + port + ",file.export=,file.reconnect-delay=30"
},
},
{
name: "block export",
req: func(t *testing.T) EBSRequest {
dev := filepath.Join(t.TempDir(), "sdx")
require.NoError(t, os.WriteFile(dev, nil, 0o600))
return EBSRequest{Name: "vol-root", BlockDevice: dev, BlockSizeBytes: 8 << 30, Boot: true}
},
wantBlockdev: func(req EBSRequest) string {
return "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=" + req.BlockDevice +
",file.aio=native,file.cache.direct=on"
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
m := directBootManager(t, &fakeNetworkPlumber{})
launches := withFakeQEMU(t)
req := tt.req(t)
instance := &VM{ID: "i-boot-blockdev", InstanceType: "t3.nano"}
instance.EBSRequests.Requests = []EBSRequest{req}
require.NoError(t, m.startQEMU(instance, false))
got := launches()
require.Len(t, got, 1)
assert.Equal(t, []string{tt.wantBlockdev(req)}, allArgValues(got[0], "-blockdev"))
assert.Contains(t, allArgValues(got[0], "-device"), "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=1,bootindex=1,werror=stop,rerror=stop")
for _, d := range allArgValues(got[0], "-drive") {
assert.NotContains(t, d, "id=os", "the boot volume must not also be a -drive")
}
})
}
}
// TestStartQEMU_DiskBoot_NoBootVolumeIsNotLaunched covers a disk-boot guest
// whose only volumes are data volumes: their -blockdev nodes would satisfy
// Execute, but QEMU would have nothing to boot from.
func TestStartQEMU_DiskBoot_NoBootVolumeIsNotLaunched(t *testing.T) {
tests := []struct {
name string
requests []EBSRequest
}{
{name: "data volume only", requests: []EBSRequest{{Name: "vol-data-a", NBDURI: "nbd:unix:/tmp/data-a.sock"}}},
{name: "no volumes", requests: nil},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
m := directBootManager(t, &fakeNetworkPlumber{})
launches := withFakeQEMU(t)
instance := &VM{ID: "i-no-boot", InstanceType: "t3.nano"}
instance.EBSRequests.Requests = tt.requests
err := m.startQEMU(instance, false)
require.Error(t, err)
assert.Contains(t, err.Error(), "no boot volume among the instance's EBS requests")
assert.Empty(t, launches(), "QEMU must not start")
})
}
}
func deviceValues(devs []Device) []string {
out := make([]string, len(devs))
for i, d := range devs {
+7 -28
View File
@@ -371,8 +371,8 @@ type Device struct {
Value string `json:"value"`
}
// Drive is one QEMU -drive entry, used for boot, EFI and pflash firmware
// drives. Each set field becomes a key=value option.
// Drive is one QEMU -drive entry, used for the pflash firmware drives (the
// CODE blob and the EFI volume). Each set field becomes a key=value option.
type Drive struct {
File string `json:"file"`
Format string `json:"format"`
@@ -380,15 +380,6 @@ type Drive struct {
Media string `json:"media"`
ID string `json:"id"`
Cache string `json:"cache,omitempty"`
// Werror and Rerror set QEMU's block on-error policy. "stop" pauses the VM
// and holds the failed request, so a transient backend loss is resumed with
// cont instead of reaching the guest as EIO and corrupting its filesystem.
Werror string `json:"werror,omitempty"`
Rerror string `json:"rerror,omitempty"`
// ReconnectDelay emits file.reconnect-delay, the seconds an NBD disconnect
// pauses requests before failing them. The file. prefix is required: the
// bare name reaches the format driver, which has no such option.
ReconnectDelay int `json:"reconnect_delay,omitempty"`
// Unit selects the pflash slot when If=="pflash": 0 is the CODE blob,
// 1 is the per-VM VARS volume. Ignored for non-pflash drives.
Unit int `json:"unit,omitempty"`
@@ -405,9 +396,9 @@ type IOThread struct {
// Blockdev is a QEMU -blockdev entry: the command-line equivalent of the
// blockdev-add QMP command. Unlike a -drive with if=none, it produces a
// monitor-owned node addressable by node-name, which is what makes
// blockdev-del able to remove it later. Used for data (non-boot, non-EFI)
// volumes so a cold-booted one has the same detachable block-graph shape as
// a hot-attached one; boot/EFI volumes keep the legacy -drive shape.
// blockdev-del able to remove it later. Used for boot and data volumes, so a
// cold-booted data volume has the same detachable block-graph shape as a
// hot-attached one; EFI volumes keep the pflash -drive shape.
type Blockdev struct {
Value string `json:"value"`
}
@@ -571,8 +562,8 @@ func (cfg *Config) Execute() (*exec.Cmd, error) {
args = append(args, "-object", fmt.Sprintf("iothread,id=%s", iot.ID))
}
if len(cfg.Drives) == 0 && cfg.KernelImage == "" {
return nil, fmt.Errorf("at least one drive or a kernel image is required")
if len(cfg.Drives) == 0 && len(cfg.Blockdevs) == 0 && cfg.KernelImage == "" {
return nil, fmt.Errorf("at least one drive, blockdev or a kernel image is required")
}
drives := cfg.Drives
@@ -622,18 +613,6 @@ func (cfg *Config) Execute() (*exec.Cmd, error) {
opts = append(opts, fmt.Sprintf("cache=%s", drive.Cache))
}
if drive.Werror != "" {
opts = append(opts, fmt.Sprintf("werror=%s", drive.Werror))
}
if drive.Rerror != "" {
opts = append(opts, fmt.Sprintf("rerror=%s", drive.Rerror))
}
if drive.ReconnectDelay > 0 {
opts = append(opts, fmt.Sprintf("file.reconnect-delay=%d", drive.ReconnectDelay))
}
args = append(args, "-drive", strings.Join(opts, ","))
}
+10 -2
View File
@@ -39,7 +39,7 @@ func TestExecute(t *testing.T) {
// Expect error, at least one drive or kernel image required
assert.Error(t, err)
assert.ErrorContains(t, err, "at least one drive or a kernel image is required")
assert.ErrorContains(t, err, "at least one drive, blockdev or a kernel image is required")
assert.Nil(t, cmd)
cfg.Drives = []Drive{
@@ -347,6 +347,14 @@ func TestExecute_DriveAndKernelInvariant(t *testing.T) {
assert.NotNil(t, cmd)
})
t.Run("blockdevs-only is OK", func(t *testing.T) {
cfg := base
cfg.Blockdevs = []Blockdev{{Value: "driver=raw,node-name=os,file.driver=nbd,file.server.type=unix,file.server.path=/run/os.sock"}}
cmd, err := cfg.Execute()
assert.NoError(t, err)
assert.NotNil(t, cmd)
})
t.Run("kernel-only is OK", func(t *testing.T) {
cfg := base
cfg.KernelImage = "/boot/vmlinuz"
@@ -359,7 +367,7 @@ func TestExecute_DriveAndKernelInvariant(t *testing.T) {
cfg := base
cmd, err := cfg.Execute()
assert.Error(t, err)
assert.ErrorContains(t, err, "at least one drive or a kernel image")
assert.ErrorContains(t, err, "at least one drive, blockdev or a kernel image")
assert.Nil(t, cmd)
})
}
+2 -3
View File
@@ -11,7 +11,6 @@ import (
"time"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/nbd"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/qmp"
)
@@ -143,7 +142,7 @@ func (m *Manager) AttachVolume(ctx context.Context, id, volumeID, device string)
if kind == volumeExportBlock {
blockdevArgs = HostDeviceOpts{Path: ebsRequest.BlockDevice, SizeBytes: ebsRequest.BlockSizeBytes}.QMPArgs(nodeName)
} else {
serverType, socketPath, nbdHost, nbdPort, err := nbd.ParseNBDURI(ebsRequest.NBDURI)
server, err := nbdServerOpts(ebsRequest.NBDURI)
if err != nil {
slog.ErrorContext(ctx, "AttachVolume: failed to parse NBDURI", "uri", ebsRequest.NBDURI, "err", err)
_ = m.rollbackUnmount(ctx, instance.AccountID, ebsRequest)
@@ -152,7 +151,7 @@ func (m *Manager) AttachVolume(ctx context.Context, id, volumeID, device string)
blockdevArgs = map[string]any{
"node-name": nodeName,
"driver": "nbd",
"server": NBDServerOpts{Type: serverType, Path: socketPath, Host: nbdHost, Port: nbdPort}.QMPArg(),
"server": server.QMPArg(),
"export": "",
"read-only": false,
"reconnect-delay": NBDReconnectDelaySeconds,