diff --git a/lib/compose/README.md b/lib/compose/README.md index 1ff7ce9..839dee9 100644 --- a/lib/compose/README.md +++ b/lib/compose/README.md @@ -37,6 +37,31 @@ services: tls: true ``` +By default, compose names resources from the compose name and service key: + +```text +instance: - +ingress: -- +``` + +Set `name` on a service or ingress rule when a stable external name is required: + +```yaml +version: 1 +name: hypeship + +services: + otelcol: + name: hypeship-otelcol-${env:DEPLOY_ENV} + image: otel/opentelemetry-collector-contrib:0.108.0 + ingress: + - name: hypeship-otelcol-${env:DEPLOY_ENV}-otlp + hostname: hypeship-otelcol-${env:DEPLOY_ENV}.dev-yul-hypeman-0.kernel.sh + host_port: 443 + target_port: 3000 + tls: true +``` + ### Commands Preview the changes: diff --git a/lib/compose/compose_test.go b/lib/compose/compose_test.go index b4dbab5..fb5ad64 100644 --- a/lib/compose/compose_test.go +++ b/lib/compose/compose_test.go @@ -23,6 +23,7 @@ func TestLoadComposeSpecInterpolatesFilesAndEnv(t *testing.T) { t.Setenv("COMPOSE_NAME", "hypeship-otel") t.Setenv("OTEL_IMAGE", "otel/opentelemetry-collector-contrib:0.108.0") t.Setenv("OTELCOL_ENV_NAME", "OTELCOL_CONFIG") + t.Setenv("DEPLOY_ENV", "staging") t.Setenv("OTEL_COLLECTOR_VM_HOSTNAME", "otel.example.com") t.Setenv("OTEL_COLLECTOR_VM_TOKEN", "collector-token") t.Setenv("SIGNOZ_ACCESS_TOKEN", "secret-token") @@ -33,13 +34,15 @@ version: 1 name: ${env:COMPOSE_NAME} services: otelcol: + name: hypeship-otelcol-${env:DEPLOY_ENV} image: ${env:OTEL_IMAGE} cmd: ["--config=env:${env:OTELCOL_ENV_NAME}"] env: OTELCOL_CONFIG: ${file:otelcol.yaml} SIGNOZ_ACCESS_TOKEN: ${env:SIGNOZ_ACCESS_TOKEN} ingress: - - hostname: ${env:OTEL_COLLECTOR_VM_HOSTNAME} + - name: hypeship-otelcol-${env:DEPLOY_ENV}-otlp + hostname: ${env:OTEL_COLLECTOR_VM_HOSTNAME} target_port: 4318 `), 0644)) @@ -48,11 +51,13 @@ services: service := spec.Services["otelcol"] assert.Equal(t, "hypeship-otel", spec.Name) + assert.Equal(t, "hypeship-otelcol-staging", service.Name) assert.Equal(t, "otel/opentelemetry-collector-contrib:0.108.0", service.Image) assert.Equal(t, []string{"--config=env:OTELCOL_CONFIG"}, service.Cmd) assert.Equal(t, "endpoint: https://otel.example.com\ntoken: collector-token\n", service.Env["OTELCOL_CONFIG"]) assert.Equal(t, "secret-token", service.Env["SIGNOZ_ACCESS_TOKEN"]) require.Len(t, service.Ingress, 1) + assert.Equal(t, "hypeship-otelcol-staging-otlp", service.Ingress[0].Name) assert.Equal(t, "otel.example.com", service.Ingress[0].Hostname) } @@ -149,6 +154,45 @@ func TestDesiredResourcesUseDeterministicNamesAndTags(t *testing.T) { assert.Equal(t, int64(4318), ingresses[0].Input.Rules[0].Target.Port) } +func TestDesiredResourcesUseExplicitResourceNames(t *testing.T) { + runner := Runner{ + spec: composeSpec{ + Version: 1, + Name: "hypeship", + Services: map[string]composeServiceSpec{ + "otelcol": { + Name: "hypeship-otelcol-staging", + Image: "otel/opentelemetry-collector-contrib:0.108.0", + Ingress: []composeIngressRuleSpec{ + { + Name: "hypeship-otelcol-staging-otlp", + Hostname: "hypeship-otelcol-staging.dev-yul-hypeman-0.kernel.sh", + HostPort: 443, + TargetPort: 3000, + TLS: true, + }, + }, + }, + }, + }, + } + + _, instances, ingresses, _, err := runner.desiredResources() + require.NoError(t, err) + + require.Len(t, instances, 1) + assert.Equal(t, "hypeship-otelcol-staging", instances[0].Name) + assert.Equal(t, "otelcol", instances[0].Service) + assert.Equal(t, "hypeship-otelcol-staging", instances[0].Input.Name) + assert.Equal(t, "otelcol", instances[0].Input.Tags[composeTagService]) + + require.Len(t, ingresses, 1) + assert.Equal(t, "hypeship-otelcol-staging-otlp", ingresses[0].Name) + assert.Equal(t, "hypeship-otelcol-staging", ingresses[0].Input.Rules[0].Target.Instance) + assert.Equal(t, int64(3000), ingresses[0].Input.Rules[0].Target.Port) + assert.Equal(t, "otelcol", ingresses[0].Input.Tags[composeTagService]) +} + func TestValidateComposeSpecRejectsInvalidNames(t *testing.T) { err := validateComposeSpec(&composeSpec{ Version: 1, @@ -161,6 +205,40 @@ func TestValidateComposeSpecRejectsInvalidNames(t *testing.T) { require.EqualError(t, err, "compose name must contain only lowercase letters, digits, and dashes") } +func TestValidateComposeSpecRejectsInvalidExplicitResourceNames(t *testing.T) { + err := validateComposeSpec(&composeSpec{ + Version: 1, + Name: "worker-stack", + Services: map[string]composeServiceSpec{ + "worker": { + Name: "BadName", + Image: "alpine:latest", + }, + }, + }) + + require.EqualError(t, err, `service "worker" name must contain only lowercase letters, digits, and dashes`) +} + +func TestValidateComposeSpecRejectsDuplicateExplicitResourceNames(t *testing.T) { + err := validateComposeSpec(&composeSpec{ + Version: 1, + Name: "worker-stack", + Services: map[string]composeServiceSpec{ + "api": { + Name: "shared-worker", + Image: "alpine:latest", + }, + "worker": { + Name: "shared-worker", + Image: "alpine:latest", + }, + }, + }) + + require.ErrorContains(t, err, `produces duplicate instance name "shared-worker"`) +} + func TestValidateComposeSpecRejectsImageAndDockerfile(t *testing.T) { err := validateComposeSpec(&composeSpec{ Version: 1, @@ -292,3 +370,109 @@ func TestConflictBlockers(t *testing.T) { require.Equal(t, []string{" instance app-api: name exists without compose ownership"}, blockers) } + +func TestPlanInstanceActionReplacesOwnedServiceWhenNameChanges(t *testing.T) { + desired := desiredInstance{ + Name: "hypeship-otelcol-production", + Service: "otelcol", + Hash: "new-hash", + Input: hypeman.InstanceNewParams{ + Name: "hypeship-otelcol-production", + }, + } + owned := []hypeman.Instance{{ + ID: "old-instance-id", + Name: "hypeship-otelcol-staging", + Tags: composeTags("hypeship", "otelcol", composeResourceInstance, "old-hash"), + }} + + action := planInstanceAction(desired, owned, nil) + + assert.Equal(t, "replace", action.Action) + assert.Equal(t, "instance", action.Type) + assert.Equal(t, "hypeship-otelcol-production", action.Name) + assert.Equal(t, "name changed from hypeship-otelcol-staging", action.Reason) + assert.Equal(t, "old-instance-id", action.instanceID) +} + +func TestPlanIngressActionReplacesOwnedServiceWhenNameChanges(t *testing.T) { + desired := desiredIngress{ + Name: "hypeship-otelcol-production-otlp", + Service: "otelcol", + Hash: "new-hash", + Input: hypeman.IngressNewParams{ + Name: "hypeship-otelcol-production-otlp", + }, + } + owned := []hypeman.Ingress{{ + ID: "old-ingress-id", + Name: "hypeship-otelcol-staging-otlp", + Tags: composeTags("hypeship", "otelcol", composeResourceIngress, "old-hash"), + }} + + action := planIngressAction(desired, owned, nil, map[string]struct{}{ + "hypeship-otelcol-production-otlp": {}, + }) + + assert.Equal(t, "replace", action.Action) + assert.Equal(t, "ingress", action.Type) + assert.Equal(t, "hypeship-otelcol-production-otlp", action.Name) + assert.Equal(t, "name changed from hypeship-otelcol-staging-otlp", action.Reason) + assert.Equal(t, "old-ingress-id", action.ingressID) +} + +func TestPlanIngressActionDoesNotReplaceIngressStillDesiredByAnotherRule(t *testing.T) { + desired := desiredIngress{ + Name: "app-api-grpc", + Service: "api", + Hash: "grpc-hash", + Input: hypeman.IngressNewParams{ + Name: "app-api-grpc", + }, + } + owned := []hypeman.Ingress{{ + ID: "http-ingress-id", + Name: "app-api-http", + Tags: composeTags("app", "api", composeResourceIngress, "http-hash"), + }} + + action := planIngressAction(desired, owned, nil, map[string]struct{}{ + "app-api-http": {}, + "app-api-grpc": {}, + }) + + assert.Equal(t, "create", action.Action) + assert.Equal(t, "missing", action.Reason) + assert.Empty(t, action.ingressID) +} + +func TestPlanIngressActionConflictsWhenRenameCandidateIsAmbiguous(t *testing.T) { + desired := desiredIngress{ + Name: "app-api-public", + Service: "api", + Hash: "public-hash", + Input: hypeman.IngressNewParams{ + Name: "app-api-public", + }, + } + owned := []hypeman.Ingress{ + { + ID: "old-http-id", + Name: "app-api-http", + Tags: composeTags("app", "api", composeResourceIngress, "http-hash"), + }, + { + ID: "old-grpc-id", + Name: "app-api-grpc", + Tags: composeTags("app", "api", composeResourceIngress, "grpc-hash"), + }, + } + + action := planIngressAction(desired, owned, nil, map[string]struct{}{ + "app-api-public": {}, + }) + + assert.Equal(t, "conflict", action.Action) + assert.Equal(t, "multiple owned ingresses for service have changed names", action.Reason) + assert.Empty(t, action.ingressID) +} diff --git a/lib/compose/desired.go b/lib/compose/desired.go index ff8aa20..37b769d 100644 --- a/lib/compose/desired.go +++ b/lib/compose/desired.go @@ -55,7 +55,7 @@ func (r *Runner) desiredResources() ([]desiredBuild, []desiredInstance, []desire builds = append(builds, build) service.Image = build.Image } - instanceName := composeInstanceName(r.spec.Name, serviceName) + instanceName := composeInstanceName(r.spec.Name, serviceName, service) instanceInput := buildComposeInstanceInput(instanceName, service) instanceHash, err := shortHash(instanceInput) if err != nil { @@ -70,7 +70,7 @@ func (r *Runner) desiredResources() ([]desiredBuild, []desiredInstance, []desire }) for i, ingressSpec := range service.Ingress { - ingressName := composeIngressName(r.spec.Name, serviceName, i) + ingressName := composeIngressName(r.spec.Name, serviceName, i, ingressSpec) ingressInput := buildComposeIngressInput(instanceName, ingressName, ingressSpec) ingressHash, err := shortHash(ingressInput) if err != nil { @@ -253,11 +253,17 @@ func buildComposeIngressInput(instanceName, ingressName string, spec composeIngr } } -func composeInstanceName(composeName, serviceName string) string { +func composeInstanceName(composeName, serviceName string, service composeServiceSpec) string { + if service.Name != "" { + return service.Name + } return composeName + "-" + serviceName } -func composeIngressName(composeName, serviceName string, index int) string { +func composeIngressName(composeName, serviceName string, index int, ingress composeIngressRuleSpec) string { + if ingress.Name != "" { + return ingress.Name + } return fmt.Sprintf("%s-%s-%d", composeName, serviceName, index) } diff --git a/lib/compose/reconcile.go b/lib/compose/reconcile.go index 215348a..570b214 100644 --- a/lib/compose/reconcile.go +++ b/lib/compose/reconcile.go @@ -59,8 +59,9 @@ func (r *Runner) Plan(ctx context.Context) (Plan, error) { if err != nil { return Plan{}, err } + desiredIngressNames := desiredIngressNamesByService(desiredIngresses) for _, ingress := range desiredIngresses { - actions = append(actions, planIngressAction(ingress, existingIngresses, *allIngresses)) + actions = append(actions, planIngressAction(ingress, existingIngresses, *allIngresses, desiredIngressNames[ingress.Service])) } return Plan{ @@ -177,7 +178,7 @@ func (r *Runner) Down(ctx context.Context, verbose bool) (Plan, error) { result.Actions = append(result.Actions, Action{ Action: "skip", Type: "ingress", - Name: composeIngressName(r.spec.Name, serviceName, i), + Name: composeIngressName(r.spec.Name, serviceName, i, service.Ingress[i]), Service: serviceName, Reason: "not found", }) @@ -185,7 +186,7 @@ func (r *Runner) Down(ctx context.Context, verbose bool) (Plan, error) { result.Actions = append(result.Actions, Action{ Action: "skip", Type: "instance", - Name: composeInstanceName(r.spec.Name, serviceName), + Name: composeInstanceName(r.spec.Name, serviceName, service), Service: serviceName, Reason: "not found", }) @@ -387,6 +388,15 @@ func planInstanceAction(desired desiredInstance, owned []hypeman.Instance, all [ } return action } + for _, inst := range owned { + if inst.Tags[composeTagService] != desired.Service || inst.Tags[composeTagResource] != composeResourceInstance { + continue + } + action.Action = "replace" + action.Reason = fmt.Sprintf("name changed from %s", inst.Name) + action.instanceID = inst.ID + return action + } for _, inst := range all { if inst.Name == desired.Name { action.Action = "conflict" @@ -400,7 +410,7 @@ func planInstanceAction(desired desiredInstance, owned []hypeman.Instance, all [ return action } -func planIngressAction(desired desiredIngress, owned []hypeman.Ingress, all []hypeman.Ingress) Action { +func planIngressAction(desired desiredIngress, owned []hypeman.Ingress, all []hypeman.Ingress, desiredServiceIngressNames map[string]struct{}) Action { action := Action{ Type: "ingress", Name: desired.Name, @@ -425,6 +435,28 @@ func planIngressAction(desired desiredIngress, owned []hypeman.Ingress, all []hy } return action } + var renameCandidates []hypeman.Ingress + for _, ing := range owned { + if ing.Tags[composeTagService] != desired.Service || ing.Tags[composeTagResource] != composeResourceIngress { + continue + } + if _, stillDesired := desiredServiceIngressNames[ing.Name]; stillDesired { + continue + } + renameCandidates = append(renameCandidates, ing) + } + if len(renameCandidates) == 1 { + ing := renameCandidates[0] + action.Action = "replace" + action.Reason = fmt.Sprintf("name changed from %s", ing.Name) + action.ingressID = ing.ID + return action + } + if len(renameCandidates) > 1 { + action.Action = "conflict" + action.Reason = "multiple owned ingresses for service have changed names" + return action + } for _, ing := range all { if ing.Name == desired.Name { action.Action = "conflict" @@ -438,6 +470,17 @@ func planIngressAction(desired desiredIngress, owned []hypeman.Ingress, all []hy return action } +func desiredIngressNamesByService(ingresses []desiredIngress) map[string]map[string]struct{} { + names := map[string]map[string]struct{}{} + for _, ingress := range ingresses { + if names[ingress.Service] == nil { + names[ingress.Service] = map[string]struct{}{} + } + names[ingress.Service][ingress.Name] = struct{}{} + } + return names +} + func (r *Runner) listComposeInstances(ctx context.Context) ([]hypeman.Instance, error) { instances, err := r.client.Instances.List(ctx, hypeman.InstanceListParams{ Tags: map[string]string{composeTagName: r.spec.Name}, diff --git a/lib/compose/spec.go b/lib/compose/spec.go index 105451a..eff08af 100644 --- a/lib/compose/spec.go +++ b/lib/compose/spec.go @@ -18,6 +18,7 @@ type composeSpec struct { } type composeServiceSpec struct { + Name string `json:"name,omitempty" yaml:"name"` Image string `json:"image" yaml:"image"` Dockerfile string `json:"dockerfile,omitempty" yaml:"dockerfile"` Entrypoint []string `json:"entrypoint,omitempty" yaml:"entrypoint"` @@ -75,6 +76,7 @@ type composeExecCheckSpec struct { } type composeIngressRuleSpec struct { + Name string `json:"name,omitempty" yaml:"name"` Hostname string `json:"hostname" yaml:"hostname"` HostPort int `json:"host_port,omitempty" yaml:"host_port"` TargetPort int `json:"target_port" yaml:"target_port"` @@ -116,14 +118,23 @@ func validateComposeSpec(spec *composeSpec) error { return fmt.Errorf("compose services must include at least one service") } + instanceNames := map[string]string{} + ingressNames := map[string]string{} for name, service := range spec.Services { if !composeNamePattern.MatchString(name) { return fmt.Errorf("service %q must contain only lowercase letters, digits, and dashes", name) } - instanceName := composeInstanceName(spec.Name, name) + instanceName := composeInstanceName(spec.Name, name, service) + if service.Name != "" && !composeNamePattern.MatchString(service.Name) { + return fmt.Errorf("service %q name must contain only lowercase letters, digits, and dashes", name) + } if len(instanceName) > 63 { return fmt.Errorf("service %q produces instance name %q longer than 63 characters", name, instanceName) } + if existing, ok := instanceNames[instanceName]; ok { + return fmt.Errorf("service %q produces duplicate instance name %q already used by service %q", name, instanceName, existing) + } + instanceNames[instanceName] = name if service.Image == "" && service.Dockerfile == "" { return fmt.Errorf("service %q image or dockerfile is required", name) } @@ -131,6 +142,14 @@ func validateComposeSpec(spec *composeSpec) error { return fmt.Errorf("service %q cannot include both image and dockerfile", name) } for i, rule := range service.Ingress { + ingressName := composeIngressName(spec.Name, name, i, rule) + if rule.Name != "" && !composeNamePattern.MatchString(rule.Name) { + return fmt.Errorf("service %q ingress %d name must contain only lowercase letters, digits, and dashes", name, i) + } + if existing, ok := ingressNames[ingressName]; ok { + return fmt.Errorf("service %q ingress %d produces duplicate ingress name %q already used by %s", name, i, ingressName, existing) + } + ingressNames[ingressName] = fmt.Sprintf("service %q ingress %d", name, i) if rule.Hostname == "" { return fmt.Errorf("service %q ingress %d hostname is required", name, i) }