Skip to content

Commit 5689831

Browse files
committed
fix: preserve referenced agent service fields
1 parent 77c0cb2 commit 5689831

7 files changed

Lines changed: 468 additions & 271 deletions

File tree

cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,9 @@ func populateContainerSettings(
734734
if err != nil {
735735
return err
736736
}
737+
if persisted == nil {
738+
return nil
739+
}
737740

738741
if err := project.AddServiceSerialized(
739742
ctx,
@@ -752,14 +755,18 @@ func prepareContainerSettings(
752755
) (*azdext.ServiceConfig, error) {
753756
rawAdditional := svc.GetAdditionalProperties()
754757
rawConfig := svc.GetConfig()
755-
if err := project.ResolveServiceConfigInPlace(
756-
svc,
757-
projectRoot,
758-
); err != nil {
759-
return nil, fmt.Errorf(
760-
"failed to resolve agent config: %w",
761-
err,
762-
)
758+
hasFileRef := project.ConfigContainsFileRef(rawAdditional) ||
759+
project.ConfigContainsFileRef(rawConfig)
760+
if hasFileRef {
761+
if err := project.ResolveServiceConfigInPlace(
762+
svc,
763+
projectRoot,
764+
); err != nil {
765+
return nil, fmt.Errorf(
766+
"failed to resolve agent config: %w",
767+
err,
768+
)
769+
}
763770
}
764771
foundryAgentConfig, err := project.LoadServiceTargetAgentConfig(svc)
765772
if err != nil {
@@ -785,19 +792,19 @@ func prepareContainerSettings(
785792
result.Cpu = project.DefaultCpu
786793
}
787794

788-
persisted := *svc
789-
persisted.AdditionalProperties = rawAdditional
790-
persisted.Config = rawConfig
791795
if err := project.SetAgentContainerSettings(
792-
&persisted,
796+
svc,
793797
&project.ContainerSettings{Resources: result},
794798
); err != nil {
795799
return nil, fmt.Errorf(
796800
"failed to update agent container settings: %w",
797801
err,
798802
)
799803
}
800-
return &persisted, nil
804+
if hasFileRef {
805+
return nil, nil
806+
}
807+
return svc, nil
801808
}
802809

803810
// resolveToolboxEnvVars resolves ${VAR} references in toolbox name, description,

cli/azd/extensions/azure.ai.agents/internal/cmd/listen_test.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ func TestIsHostedAgentServiceRejectsTraversal(t *testing.T) {
131131
}
132132
}
133133

134-
func TestPrepareContainerSettings_ResolvesFileRef(t *testing.T) {
134+
func TestPopulateContainerSettings_DoesNotPersistResolvedFileRef(
135+
t *testing.T,
136+
) {
135137
t.Parallel()
136138

137139
root := t.TempDir()
@@ -158,17 +160,11 @@ func TestPrepareContainerSettings_ResolvesFileRef(t *testing.T) {
158160
AdditionalProperties: props,
159161
}
160162

161-
persisted, err := prepareContainerSettings(svc, root)
163+
err = populateContainerSettings(t.Context(), nil, svc, root)
162164

163165
require.NoError(t, err)
164166
require.Equal(t, "src/echo", svc.GetRelativePath())
165-
require.Equal(t, "src/echo", persisted.GetRelativePath())
166-
require.Contains(
167-
t,
168-
persisted.GetAdditionalProperties().GetFields(),
169-
"$ref",
170-
)
171-
cfg, err := project.LoadServiceTargetAgentConfig(persisted)
167+
cfg, err := project.LoadServiceTargetAgentConfig(svc)
172168
require.NoError(t, err)
173169
require.NotNil(t, cfg.Container)
174170
require.NotNil(t, cfg.Container.Resources)

cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go

Lines changed: 89 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
"github.com/azure/azure-dev/cli/azd/pkg/output"
4242
"github.com/fatih/color"
4343
"github.com/google/uuid"
44+
"google.golang.org/protobuf/proto"
4445
"google.golang.org/protobuf/types/known/structpb"
4546
)
4647

@@ -146,8 +147,7 @@ var _ azdext.ServiceTargetProvider = &AgentServiceTargetProvider{}
146147
type AgentServiceTargetProvider struct {
147148
azdClient *azdext.AzdClient
148149
serviceConfig *azdext.ServiceConfig
149-
sourceProperties *structpb.Struct
150-
sourceConfig *structpb.Struct
150+
sourceServiceConfig *azdext.ServiceConfig
151151
agentDefinitionPath string
152152
projectPath string
153153
servicePath string
@@ -185,8 +185,7 @@ func NewAgentServiceTargetProvider(azdClient *azdext.AzdClient) azdext.ServiceTa
185185
// only when a deploy-time entrypoint needs it.
186186
func (p *AgentServiceTargetProvider) Initialize(ctx context.Context, serviceConfig *azdext.ServiceConfig) error {
187187
p.serviceConfig = serviceConfig
188-
p.sourceProperties = serviceConfig.GetAdditionalProperties()
189-
p.sourceConfig = serviceConfig.GetConfig()
188+
p.sourceServiceConfig = proto.Clone(serviceConfig).(*azdext.ServiceConfig)
190189
return nil
191190
}
192191

@@ -545,7 +544,7 @@ func (p *AgentServiceTargetProvider) Package(
545544
serviceConfig *azdext.ServiceConfig,
546545
serviceContext *azdext.ServiceContext,
547546
progress azdext.ProgressReporter,
548-
) (*azdext.ServicePackageResult, error) {
547+
) (result *azdext.ServicePackageResult, err error) {
549548
if err := p.ensureDeployContext(ctx); err != nil {
550549
return nil, err
551550
}
@@ -590,12 +589,26 @@ func (p *AgentServiceTargetProvider) Package(
590589
Artifacts: []*azdext.Artifact{preBuiltImageArtifact(agentDef.Image)},
591590
}, nil
592591
}
593-
if err := p.persistBuildService(ctx); err != nil {
592+
restore, err := p.persistBuildService(ctx)
593+
if err != nil {
594594
return nil, exterrors.Internal(
595595
exterrors.OpContainerBuild,
596596
fmt.Sprintf("prepare referenced service: %s", err),
597597
)
598598
}
599+
if restore != nil {
600+
defer func() {
601+
err = combineBuildServiceRestore(
602+
ctx,
603+
restore,
604+
err,
605+
exterrors.OpContainerBuild,
606+
)
607+
if err != nil {
608+
result = nil
609+
}
610+
}()
611+
}
599612

600613
var packageArtifact *azdext.Artifact
601614
var newArtifacts []*azdext.Artifact
@@ -651,25 +664,61 @@ func (p *AgentServiceTargetProvider) Package(
651664

652665
func (p *AgentServiceTargetProvider) persistBuildService(
653666
ctx context.Context,
654-
) error {
655-
if !configContainsFileRef(p.sourceProperties) &&
656-
!configContainsFileRef(p.sourceConfig) {
657-
return nil
658-
}
659-
persisted := *p.serviceConfig
660-
persisted.AdditionalProperties = p.sourceProperties
661-
persisted.Config = p.sourceConfig
667+
) (func(context.Context) error, error) {
668+
source := p.sourceServiceConfig
669+
if source == nil ||
670+
(!ConfigContainsFileRef(source.GetAdditionalProperties()) &&
671+
!ConfigContainsFileRef(source.GetConfig())) {
672+
return nil, nil
673+
}
674+
resolvedPath := p.serviceConfig.GetRelativePath()
675+
if resolvedPath == source.GetRelativePath() {
676+
return nil, nil
677+
}
678+
persisted := proto.Clone(source).(*azdext.ServiceConfig)
679+
persisted.RelativePath = resolvedPath
662680
if err := AddServiceSerialized(
663681
ctx,
664682
p.azdClient,
665-
&persisted,
683+
persisted,
666684
); err != nil {
667-
return fmt.Errorf("persist resolved service fields: %w", err)
685+
return nil, fmt.Errorf("persist resolved service fields: %w", err)
668686
}
669-
return nil
687+
return func(restoreCtx context.Context) error {
688+
original := proto.Clone(source).(*azdext.ServiceConfig)
689+
if err := AddServiceSerialized(
690+
restoreCtx,
691+
p.azdClient,
692+
original,
693+
); err != nil {
694+
return fmt.Errorf("restore referenced service: %w", err)
695+
}
696+
return nil
697+
}, nil
670698
}
671699

672-
func configContainsFileRef(config *structpb.Struct) bool {
700+
func combineBuildServiceRestore(
701+
ctx context.Context,
702+
restore func(context.Context) error,
703+
operationErr error,
704+
code string,
705+
) error {
706+
if restore == nil {
707+
return operationErr
708+
}
709+
restoreErr := restore(context.WithoutCancel(ctx))
710+
if restoreErr == nil {
711+
return operationErr
712+
}
713+
classified := exterrors.Internal(code, restoreErr.Error())
714+
if operationErr == nil {
715+
return classified
716+
}
717+
return errors.Join(operationErr, classified)
718+
}
719+
720+
// ConfigContainsFileRef reports whether config contains a local $ref.
721+
func ConfigContainsFileRef(config *structpb.Struct) bool {
673722
if config == nil {
674723
return false
675724
}
@@ -703,7 +752,7 @@ func (p *AgentServiceTargetProvider) Publish(
703752
targetResource *azdext.TargetResource,
704753
publishOptions *azdext.PublishOptions,
705754
progress azdext.ProgressReporter,
706-
) (*azdext.ServicePublishResult, error) {
755+
) (result *azdext.ServicePublishResult, err error) {
707756
// Pre-built image: nothing to package or push. Skip deploy-context
708757
// resolution so this path stays cheap and doesn't require agent.yaml.
709758
if preBuiltArtifact := findPreBuiltImageArtifact(serviceContext.Package); preBuiltArtifact != nil {
@@ -729,6 +778,27 @@ func (p *AgentServiceTargetProvider) Publish(
729778
return &azdext.ServicePublishResult{}, nil
730779
}
731780

781+
restore, err := p.persistBuildService(ctx)
782+
if err != nil {
783+
return nil, exterrors.Internal(
784+
exterrors.OpContainerPublish,
785+
fmt.Sprintf("prepare referenced service: %s", err),
786+
)
787+
}
788+
if restore != nil {
789+
defer func() {
790+
err = combineBuildServiceRestore(
791+
ctx,
792+
restore,
793+
err,
794+
exterrors.OpContainerPublish,
795+
)
796+
if err != nil {
797+
result = nil
798+
}
799+
}()
800+
}
801+
732802
progress("Publishing container")
733803
publishResponse, err := p.azdClient.
734804
Container().

0 commit comments

Comments
 (0)