feat(container): loadEnvironment skips EnvLoader when Route.Env is pre-resolved
Add preResolved []string parameter to loadEnvironment; when non-empty, use those env vars directly instead of calling envLoader.LoadEnv(). Update all 3 call sites and add two unit tests covering both code paths.
- commit
- c2b0666d7387642dc09afcb98402c3a3fe298672
- author
- brice · · 2026-03-18 18:34 UTC
- committer
- brice · · 2026-03-18 17:15 UTC
- parent
- a18600c
- signature
- unsigned
2 files · +54 -7
M internal/usecase/container/service.go +13 -7
| @@ -568,7 +568,7 @@ func (s *Service) prepareDeployResources(ctx context.Context, route domain.Route | |||
| 568 | 568 | imageLabels = nil | |
| 569 | 569 | } | |
| 570 | 570 | ||
| 571 | - | envVars, err := s.loadEnvironment(ctx, route.Domain, actualImageRef) | |
| 571 | + | envVars, err := s.loadEnvironment(ctx, route.Env, route.Domain, actualImageRef) | |
| 572 | 572 | if err != nil { | |
| 573 | 573 | return nil, err | |
| 574 | 574 | } | |
| @@ -1774,12 +1774,18 @@ func (s *Service) tagImageIfNeeded(ctx context.Context, sourceRef, targetRef str | |||
| 1774 | 1774 | return nil | |
| 1775 | 1775 | } | |
| 1776 | 1776 | ||
| 1777 | - | func (s *Service) loadEnvironment(ctx context.Context, domainName, imageRef string) ([]string, error) { | |
| 1777 | + | func (s *Service) loadEnvironment(ctx context.Context, preResolved []string, domainName, imageRef string) ([]string, error) { | |
| 1778 | 1778 | log := zerowrap.FromCtx(ctx) | |
| 1779 | 1779 | ||
| 1780 | - | userEnvVars, err := s.envLoader.LoadEnv(ctx, domainName) | |
| 1781 | - | if err != nil { | |
| 1782 | - | return nil, log.WrapErr(err, "failed to load environment variables") | |
| 1780 | + | var userEnvVars []string | |
| 1781 | + | if len(preResolved) > 0 { | |
| 1782 | + | userEnvVars = preResolved | |
| 1783 | + | } else { | |
| 1784 | + | var err error | |
| 1785 | + | userEnvVars, err = s.envLoader.LoadEnv(ctx, domainName) | |
| 1786 | + | if err != nil { | |
| 1787 | + | return nil, log.WrapErr(err, "failed to load environment variables") | |
| 1788 | + | } | |
| 1783 | 1789 | } | |
| 1784 | 1790 | ||
| 1785 | 1791 | dockerfileEnvVars, err := s.runtime.InspectImageEnv(ctx, imageRef) | |
| @@ -2219,7 +2225,7 @@ func (s *Service) deployAttachedService(ctx context.Context, ownerDomain, servic | |||
| 2219 | 2225 | } | |
| 2220 | 2226 | ||
| 2221 | 2227 | // Load environment (attachment-specific env file) | |
| 2222 | - | envVars, err := s.loadEnvironment(ctx, containerName, actualImageRef) | |
| 2228 | + | envVars, err := s.loadEnvironment(ctx, nil, containerName, actualImageRef) | |
| 2223 | 2229 | if err != nil { | |
| 2224 | 2230 | log.WrapErr(err, "failed to load environment for attachment") | |
| 2225 | 2231 | envVars = []string{} | |
| @@ -2268,7 +2274,7 @@ func (s *Service) deployAttachedService(ctx context.Context, ownerDomain, servic | |||
| 2268 | 2274 | } | |
| 2269 | 2275 | ||
| 2270 | 2276 | func (s *Service) attachmentEnvDrifted(ctx context.Context, existing *domain.Container, containerName, serviceImage string) (bool, error) { | |
| 2271 | - | currentEnv, err := s.loadEnvironment(ctx, containerName, s.buildImageRef(serviceImage)) | |
| 2277 | + | currentEnv, err := s.loadEnvironment(ctx, nil, containerName, s.buildImageRef(serviceImage)) | |
| 2272 | 2278 | if err != nil { | |
| 2273 | 2279 | return false, err | |
| 2274 | 2280 | } | |
| @@ -568,7 +568,7 @@ func (s *Service) prepareDeployResources(ctx context.Context, route domain.Route | |||||
| 568 | imageLabels = nil | 568 | imageLabels = nil | ||
| 569 | } | 569 | } | ||
| 570 | 570 | ||||
| 571 | - | envVars, err := s.loadEnvironment(ctx, route.Domain, actualImageRef) | 571 | + | envVars, err := s.loadEnvironment(ctx, route.Env, route.Domain, actualImageRef) |
| 572 | if err != nil { | 572 | if err != nil { | ||
| 573 | return nil, err | 573 | return nil, err | ||
| 574 | } | 574 | } | ||
| @@ -1774,12 +1774,18 @@ func (s *Service) tagImageIfNeeded(ctx context.Context, sourceRef, targetRef str | |||||
| 1774 | return nil | 1774 | return nil | ||
| 1775 | } | 1775 | } | ||
| 1776 | 1776 | ||||
| 1777 | - | func (s *Service) loadEnvironment(ctx context.Context, domainName, imageRef string) ([]string, error) { | 1777 | + | func (s *Service) loadEnvironment(ctx context.Context, preResolved []string, domainName, imageRef string) ([]string, error) { |
| 1778 | log := zerowrap.FromCtx(ctx) | 1778 | log := zerowrap.FromCtx(ctx) | ||
| 1779 | 1779 | ||||
| 1780 | - | userEnvVars, err := s.envLoader.LoadEnv(ctx, domainName) | 1780 | + | var userEnvVars []string |
| 1781 | - | if err != nil { | 1781 | + | if len(preResolved) > 0 { |
| 1782 | - | return nil, log.WrapErr(err, "failed to load environment variables") | 1782 | + | userEnvVars = preResolved |
| 1783 | + | } else { | |||
| 1784 | + | var err error | |||
| 1785 | + | userEnvVars, err = s.envLoader.LoadEnv(ctx, domainName) | |||
| 1786 | + | if err != nil { | |||
| 1787 | + | return nil, log.WrapErr(err, "failed to load environment variables") | |||
| 1788 | + | } | |||
| 1783 | } | 1789 | } | ||
| 1784 | 1790 | ||||
| 1785 | dockerfileEnvVars, err := s.runtime.InspectImageEnv(ctx, imageRef) | 1791 | dockerfileEnvVars, err := s.runtime.InspectImageEnv(ctx, imageRef) | ||
| @@ -2219,7 +2225,7 @@ func (s *Service) deployAttachedService(ctx context.Context, ownerDomain, servic | |||||
| 2219 | } | 2225 | } | ||
| 2220 | 2226 | ||||
| 2221 | // Load environment (attachment-specific env file) | 2227 | // Load environment (attachment-specific env file) | ||
| 2222 | - | envVars, err := s.loadEnvironment(ctx, containerName, actualImageRef) | 2228 | + | envVars, err := s.loadEnvironment(ctx, nil, containerName, actualImageRef) |
| 2223 | if err != nil { | 2229 | if err != nil { | ||
| 2224 | log.WrapErr(err, "failed to load environment for attachment") | 2230 | log.WrapErr(err, "failed to load environment for attachment") | ||
| 2225 | envVars = []string{} | 2231 | envVars = []string{} | ||
| @@ -2268,7 +2274,7 @@ func (s *Service) deployAttachedService(ctx context.Context, ownerDomain, servic | |||||
| 2268 | } | 2274 | } | ||
| 2269 | 2275 | ||||
| 2270 | func (s *Service) attachmentEnvDrifted(ctx context.Context, existing *domain.Container, containerName, serviceImage string) (bool, error) { | 2276 | func (s *Service) attachmentEnvDrifted(ctx context.Context, existing *domain.Container, containerName, serviceImage string) (bool, error) { | ||
| 2271 | - | currentEnv, err := s.loadEnvironment(ctx, containerName, s.buildImageRef(serviceImage)) | 2277 | + | currentEnv, err := s.loadEnvironment(ctx, nil, containerName, s.buildImageRef(serviceImage)) |
| 2272 | if err != nil { | 2278 | if err != nil { | ||
| 2273 | return false, err | 2279 | return false, err | ||
| 2274 | } | 2280 | } | ||
M internal/usecase/container/service_test.go +41 -0
| @@ -3794,3 +3794,44 @@ func TestService_Deploy_PropagatesImageLabelsToContainerConfig(t *testing.T) { | |||
| 3794 | 3794 | assert.Equal(t, "c-gitea-123", result.ID) | |
| 3795 | 3795 | assert.Equal(t, "running", result.Status) | |
| 3796 | 3796 | } | |
| 3797 | + | ||
| 3798 | + | func TestLoadEnvironment_PreResolvedEnv(t *testing.T) { | |
| 3799 | + | ctx := testContext() | |
| 3800 | + | ||
| 3801 | + | mockEnvLoader := mocks.NewMockEnvLoader(t) | |
| 3802 | + | mockRuntime := mocks.NewMockContainerRuntime(t) | |
| 3803 | + | ||
| 3804 | + | // EnvLoader should NOT be called when pre-resolved env is provided | |
| 3805 | + | mockRuntime.EXPECT().InspectImageEnv(ctx, "myapp:latest"). | |
| 3806 | + | Return([]string{"FROM_DOCKERFILE=base"}, nil) | |
| 3807 | + | ||
| 3808 | + | svc := NewService(mockRuntime, mockEnvLoader, mocks.NewMockEventPublisher(t), nil, Config{}, nil) | |
| 3809 | + | ||
| 3810 | + | preResolved := []string{"DB_HOST=localhost", "API_KEY=secret"} | |
| 3811 | + | result, err := svc.loadEnvironment(ctx, preResolved, "ignored-domain", "myapp:latest") | |
| 3812 | + | require.NoError(t, err) | |
| 3813 | + | ||
| 3814 | + | assert.Contains(t, result, "DB_HOST=localhost") | |
| 3815 | + | assert.Contains(t, result, "API_KEY=secret") | |
| 3816 | + | assert.Contains(t, result, "FROM_DOCKERFILE=base") | |
| 3817 | + | } | |
| 3818 | + | ||
| 3819 | + | func TestLoadEnvironment_NoPreResolvedEnv(t *testing.T) { | |
| 3820 | + | ctx := testContext() | |
| 3821 | + | ||
| 3822 | + | mockEnvLoader := mocks.NewMockEnvLoader(t) | |
| 3823 | + | mockRuntime := mocks.NewMockContainerRuntime(t) | |
| 3824 | + | ||
| 3825 | + | mockEnvLoader.EXPECT().LoadEnv(ctx, "myapp.example.com"). | |
| 3826 | + | Return([]string{"DB_HOST=prod"}, nil) | |
| 3827 | + | mockRuntime.EXPECT().InspectImageEnv(ctx, "myapp:latest"). | |
| 3828 | + | Return([]string{"FROM_DOCKERFILE=base"}, nil) | |
| 3829 | + | ||
| 3830 | + | svc := NewService(mockRuntime, mockEnvLoader, mocks.NewMockEventPublisher(t), nil, Config{}, nil) | |
| 3831 | + | ||
| 3832 | + | result, err := svc.loadEnvironment(ctx, nil, "myapp.example.com", "myapp:latest") | |
| 3833 | + | require.NoError(t, err) | |
| 3834 | + | ||
| 3835 | + | assert.Contains(t, result, "DB_HOST=prod") | |
| 3836 | + | assert.Contains(t, result, "FROM_DOCKERFILE=base") | |
| 3837 | + | } | |
| @@ -3794,3 +3794,44 @@ func TestService_Deploy_PropagatesImageLabelsToContainerConfig(t *testing.T) { | |||||
| 3794 | assert.Equal(t, "c-gitea-123", result.ID) | 3794 | assert.Equal(t, "c-gitea-123", result.ID) | ||
| 3795 | assert.Equal(t, "running", result.Status) | 3795 | assert.Equal(t, "running", result.Status) | ||
| 3796 | } | 3796 | } | ||
| 3797 | + | ||||
| 3798 | + | func TestLoadEnvironment_PreResolvedEnv(t *testing.T) { | |||
| 3799 | + | ctx := testContext() | |||
| 3800 | + | ||||
| 3801 | + | mockEnvLoader := mocks.NewMockEnvLoader(t) | |||
| 3802 | + | mockRuntime := mocks.NewMockContainerRuntime(t) | |||
| 3803 | + | ||||
| 3804 | + | // EnvLoader should NOT be called when pre-resolved env is provided | |||
| 3805 | + | mockRuntime.EXPECT().InspectImageEnv(ctx, "myapp:latest"). | |||
| 3806 | + | Return([]string{"FROM_DOCKERFILE=base"}, nil) | |||
| 3807 | + | ||||
| 3808 | + | svc := NewService(mockRuntime, mockEnvLoader, mocks.NewMockEventPublisher(t), nil, Config{}, nil) | |||
| 3809 | + | ||||
| 3810 | + | preResolved := []string{"DB_HOST=localhost", "API_KEY=secret"} | |||
| 3811 | + | result, err := svc.loadEnvironment(ctx, preResolved, "ignored-domain", "myapp:latest") | |||
| 3812 | + | require.NoError(t, err) | |||
| 3813 | + | ||||
| 3814 | + | assert.Contains(t, result, "DB_HOST=localhost") | |||
| 3815 | + | assert.Contains(t, result, "API_KEY=secret") | |||
| 3816 | + | assert.Contains(t, result, "FROM_DOCKERFILE=base") | |||
| 3817 | + | } | |||
| 3818 | + | ||||
| 3819 | + | func TestLoadEnvironment_NoPreResolvedEnv(t *testing.T) { | |||
| 3820 | + | ctx := testContext() | |||
| 3821 | + | ||||
| 3822 | + | mockEnvLoader := mocks.NewMockEnvLoader(t) | |||
| 3823 | + | mockRuntime := mocks.NewMockContainerRuntime(t) | |||
| 3824 | + | ||||
| 3825 | + | mockEnvLoader.EXPECT().LoadEnv(ctx, "myapp.example.com"). | |||
| 3826 | + | Return([]string{"DB_HOST=prod"}, nil) | |||
| 3827 | + | mockRuntime.EXPECT().InspectImageEnv(ctx, "myapp:latest"). | |||
| 3828 | + | Return([]string{"FROM_DOCKERFILE=base"}, nil) | |||
| 3829 | + | ||||
| 3830 | + | svc := NewService(mockRuntime, mockEnvLoader, mocks.NewMockEventPublisher(t), nil, Config{}, nil) | |||
| 3831 | + | ||||
| 3832 | + | result, err := svc.loadEnvironment(ctx, nil, "myapp.example.com", "myapp:latest") | |||
| 3833 | + | require.NoError(t, err) | |||
| 3834 | + | ||||
| 3835 | + | assert.Contains(t, result, "DB_HOST=prod") | |||
| 3836 | + | assert.Contains(t, result, "FROM_DOCKERFILE=base") | |||
| 3837 | + | } | |||