chore: Interpolate slice of strings in the manifest

[KollaAdithya]

closes #4928

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

[github-actions]

🍕 Here are the new binary sizes!

Name	New size (kiB)	size (kiB)	Delta (%)
macOS (amd)	51072	50748	+0.64
macOS (arm)	51260	50956	+0.60
linux (amd)	44960	44680	+0.63
linux (arm)	43268	43012	+0.60
windows (amd)	41800	41548	+0.61

[codecov-commenter]

Codecov Report

Merging #4993 (25d7b2d) into mainline (399658b) will increase coverage by 0.20%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##           mainline    #4993      +/-   ##
============================================
+ Coverage     70.15%   70.36%   +0.20%     
============================================
  Files           289      290       +1     
  Lines         41797    42446     +649     
  Branches        285      285              
============================================
+ Hits          29324    29868     +544     
- Misses        11060    11157      +97     
- Partials       1413     1421       +8     

Impacted Files	Coverage Δ
internal/pkg/manifest/interpolate.go	83.87% <100.00%> (+2.85%)	⬆️

... and 25 files with indirect coverage changes

[iamhopaul123]

We might need to think about more scenarios: right now it works for both string and list. However, it won't work for something like

image:
  build: api/Dockerfile
  port: 8080
  healthcheck: ${HEALTHCHECK}

where

HEALTHCHECK={command: ["CMD-SHELL", "curl -f http://localhost:8080/_healthcheck || exit 1"]}

Maybe instead of doing the unmarshaling and operate in the yaml node level, we could just do string substitution?

[KollaAdithya]

Maybe instead of doing the unmarshaling and operate in the yaml node level, we could just do string substitution?

This is good option as well. we can just replace all the occurences of the ${ } in the manifest.
But only the downside is they can now interpolate the manifest fields as well.
${security_group_field_name}: [sg1,sg2,sg3]. But there is no reason why they would do manifest field interpolation.

[iamhopaul123]

${security_group_field_name}: [sg1,2,3]. But there is no reason why they would do manifest field interpolation.

Yeah I guess that's true...Open to other thoughts too.

[dannyrandall]

maybe i'm misunderstanding something....why don't we just interoplate at the ReadWorkloadManifest() level?

copilot-cli/internal/pkg/workspace/workspace.go

Lines 350 to 360 in b408b6d

	func (ws *Workspace) ReadWorkloadManifest(mftDirName string) (WorkloadManifest, error) {
	raw, err := ws.read(mftDirName, manifestFileName)
	if err != nil {
	return nil, err
	}
	mft := WorkloadManifest(raw)
	if err := ws.manifestNameMatchWithDir(mft, mftDirName); err != nil {
	return nil, err
	}
	return mft, nil
	}

So essentially the flow would be read manifest bytes -> interpolate on manifest string -> yaml.Unmarshal

[KollaAdithya]

After a offline discussion agreed on that we support only list and string interpolation for now. If any more use cases come up for the whole manifest substitution or map interpolation. we will implement that

# "svc" manifest
${my_manifest}

[iamhopaul123]

Looks good to me overall. Can we update the doc for this as well? Just a callout this seems to only work for "slice of string" instead of general "slice of anything", which seems good to me.

[bvtujo]

slick! Looks good to me.