Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(template): Introduce new File and InMemory Templates #1197

Merged
merged 5 commits into from
Oct 5, 2023
Merged

refactor(template): Introduce new File and InMemory Templates #1197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a05a478
refactor(template): Introduce new File and InMemory Templates
UnseenWizzard Sep 22, 2023
3bf2bfd
refactor(download): Handle template errors in dependency resolution
UnseenWizzard Sep 22, 2023
c808f40
fix: Fix error formatting of path
Laubi Sep 25, 2023
8d4e57e
docs: Describe Template methods and fields in comments
UnseenWizzard Oct 2, 2023
a4511cf
refactor(persistence): Remove logic to persist templates that are nev…
UnseenWizzard Oct 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions cmd/monaco/download/download_configs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,11 +212,17 @@ func doDownloadConfigs(fs afero.Fs, downloaders downloaders, opts downloadConfig
}

log.Info("Resolving dependencies between configurations")
downloadedConfigs = dependency_resolution.ResolveDependencies(downloadedConfigs)
downloadedConfigs, err = dependency_resolution.ResolveDependencies(downloadedConfigs)
if err != nil {
return err
}

log.Info("Extracting additional identifiers into YAML parameters")
// must happen after dep-resolution, as it removes IDs from the JSONs in which the dep-resolution searches as well
downloadedConfigs = id_extraction.ExtractIDsIntoYAML(downloadedConfigs)
downloadedConfigs, err = id_extraction.ExtractIDsIntoYAML(downloadedConfigs)
if err != nil {
return err
}

return writeConfigs(downloadedConfigs, opts.downloadOptionsShared, fs)
}
Expand Down
74 changes: 27 additions & 47 deletions cmd/monaco/download/download_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ import (
// We want to be pragmatic in comparing them - so we define these options to make it very simple.
var compareOptions = []cmp.Option{
cmp.Comparer(func(a, b template.Template) bool {
return jsonEqual(a.Content(), b.Content())
cA, _ := a.Content()
cB, _ := b.Content()
return jsonEqual(cA, cB)
}),
cmpopts.SortSlices(func(a, b config.Config) bool {
return strings.Compare(a.Coordinate.String(), b.Coordinate.String()) < 0
Expand All @@ -59,28 +61,6 @@ var compareOptions = []cmp.Option{
}),
}

type contentOnlyTemplate struct {
content string
}

func (c contentOnlyTemplate) Id() string {
panic("implement me")
}

func (c contentOnlyTemplate) Name() string {
panic("implement me")
}

func (c contentOnlyTemplate) Content() string {
return c.content
}

func (c contentOnlyTemplate) UpdateContent(_ string) {
panic("implement me")
}

var _ template.Template = (*contentOnlyTemplate)(nil)

func TestDownloadIntegrationSimple(t *testing.T) {
// GIVEN apis, server responses, file system
const projectName = "integration-test-1"
Expand Down Expand Up @@ -142,7 +122,7 @@ func TestDownloadIntegrationSimple(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("template-1", `{"custom-response": true, "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: fakeApi.ID},
},
},
Expand Down Expand Up @@ -208,7 +188,7 @@ func TestDownloadIntegrationWithReference(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("template-1", `{"custom-response": true, "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: "fake-id"},
},
{
Expand All @@ -220,7 +200,7 @@ func TestDownloadIntegrationWithReference(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}", "reference-to-id1": "{{.fakeid__id1__id}}"}`},
Template: template.NewInMemoryTemplate("template-2", `{"custom-response": true, "name": "{{.name}}", "reference-to-id1": "{{.fakeid__id1__id}}"}`),
Type: config.ClassicApiType{Api: "fake-id"},
},
},
Expand Down Expand Up @@ -295,7 +275,7 @@ func TestDownloadIntegrationWithMultipleApisAndReferences(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": true, "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: "fake-id-1"},
},
{
Expand All @@ -307,7 +287,7 @@ func TestDownloadIntegrationWithMultipleApisAndReferences(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": false, "name": "{{.name}}", "reference-to-id1": "{{.fakeid1__id1__id}}"}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": false, "name": "{{.name}}", "reference-to-id1": "{{.fakeid1__id1__id}}"}`),
Type: config.ClassicApiType{Api: "fake-id-1"},
},
},
Expand All @@ -321,7 +301,7 @@ func TestDownloadIntegrationWithMultipleApisAndReferences(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": "No!", "name": "{{.name}}", "subobject": {"something": "{{.fakeid1__id1__id}}"}}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": "No!", "name": "{{.name}}", "subobject": {"something": "{{.fakeid1__id1__id}}"}}`),
Type: config.ClassicApiType{Api: "fake-id-2"},
},
{
Expand All @@ -333,7 +313,7 @@ func TestDownloadIntegrationWithMultipleApisAndReferences(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}", "reference-to-id3": "{{.fakeid2__id3__id}}"}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": true, "name": "{{.name}}", "reference-to-id3": "{{.fakeid2__id3__id}}"}`),
Type: config.ClassicApiType{Api: "fake-id-2"},
},
},
Expand All @@ -348,7 +328,7 @@ func TestDownloadIntegrationWithMultipleApisAndReferences(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"name": "{{.name}}", "custom-response": true, "reference-to-id6-of-another-api": ["{{.fakeid2__id4__id}}" ,{"o": "{{.fakeid1__id2__id}}"}]}`},
Template: template.NewInMemoryTemplate("id", `{"name": "{{.name}}", "custom-response": true, "reference-to-id6-of-another-api": ["{{.fakeid2__id4__id}}" ,{"o": "{{.fakeid1__id2__id}}"}]}`),
Type: config.ClassicApiType{Api: "fake-id-3"},
},
},
Expand Down Expand Up @@ -413,7 +393,7 @@ func TestDownloadIntegrationSingletonConfig(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": true, "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: "fake-id"},
},
},
Expand Down Expand Up @@ -479,7 +459,7 @@ func TestDownloadIntegrationSyntheticLocations(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"type": "PRIVATE", "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("id", `{"type": "PRIVATE", "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: "synthetic-location"},
},
},
Expand Down Expand Up @@ -549,7 +529,7 @@ func TestDownloadIntegrationDashboards(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}", "owner": "Q"}, "tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}", "owner": "Q"}, "tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -560,7 +540,7 @@ func TestDownloadIntegrationDashboards(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}", "owner": "Admiral Jean-Luc Picard"}, "tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}", "owner": "Admiral Jean-Luc Picard"}, "tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -571,7 +551,7 @@ func TestDownloadIntegrationDashboards(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}","owner": "Not Dynatrace","preset": true},"tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}","owner": "Not Dynatrace","preset": true},"tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
},
Expand Down Expand Up @@ -644,7 +624,7 @@ func TestDownloadIntegrationAllDashboardsAreDownloadedIfFilterFFTurnedOff(t *tes
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}", "owner": "Q"}, "tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}", "owner": "Q"}, "tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -655,7 +635,7 @@ func TestDownloadIntegrationAllDashboardsAreDownloadedIfFilterFFTurnedOff(t *tes
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}", "owner": "Admiral Jean-Luc Picard"}, "tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}", "owner": "Admiral Jean-Luc Picard"}, "tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -666,7 +646,7 @@ func TestDownloadIntegrationAllDashboardsAreDownloadedIfFilterFFTurnedOff(t *tes
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}","owner": "Dynatrace"},"tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}","owner": "Dynatrace"},"tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -677,7 +657,7 @@ func TestDownloadIntegrationAllDashboardsAreDownloadedIfFilterFFTurnedOff(t *tes
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}","owner": "Not Dynatrace","preset": true},"tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}","owner": "Not Dynatrace","preset": true},"tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
{
Expand All @@ -688,7 +668,7 @@ func TestDownloadIntegrationAllDashboardsAreDownloadedIfFilterFFTurnedOff(t *tes
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"dashboardMetadata": {"name": "{{.name}}","owner": "Dynatrace","preset": true},"tiles": []}`},
Template: template.NewInMemoryTemplate("id", `{"dashboardMetadata": {"name": "{{.name}}","owner": "Dynatrace","preset": true},"tiles": []}`),
Type: config.ClassicApiType{Api: "dashboard"},
},
},
Expand Down Expand Up @@ -755,7 +735,7 @@ func TestDownloadIntegrationAnomalyDetectionMetrics(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{}`},
Template: template.NewInMemoryTemplate("id", `{}`),
Type: config.ClassicApiType{Api: "anomaly-detection-metrics"},
},
{
Expand All @@ -766,7 +746,7 @@ func TestDownloadIntegrationAnomalyDetectionMetrics(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{}`},
Template: template.NewInMemoryTemplate("id", `{}`),
Type: config.ClassicApiType{Api: "anomaly-detection-metrics"},
},
},
Expand All @@ -791,7 +771,7 @@ func TestDownloadIntegrationHostAutoUpdate(t *testing.T) {
},
Group: "default",
Environment: "valid",
Template: contentOnlyTemplate{`{"updateWindows":{"windows":[{"id":"3","name":"Daily maintenance window"}]}}`},
Template: template.NewInMemoryTemplate("id", `{"updateWindows":{"windows":[{"id":"3","name":"Daily maintenance window"}]}}`),
Type: config.ClassicApiType{Api: "hosts-auto-update"},
},
},
Expand All @@ -808,7 +788,7 @@ func TestDownloadIntegrationHostAutoUpdate(t *testing.T) {
},
Group: "default",
Environment: "updateWindows-empty",
Template: contentOnlyTemplate{`{}`},
Template: template.NewInMemoryTemplate("id", `{}`),
Type: config.ClassicApiType{Api: "hosts-auto-update"},
},
},
Expand All @@ -830,7 +810,7 @@ func TestDownloadIntegrationHostAutoUpdate(t *testing.T) {
},
Group: "default",
Environment: "windows-missing",
Template: contentOnlyTemplate{`{"updateWindows":{}}`},
Template: template.NewInMemoryTemplate("id", `{"updateWindows":{}}`),
Type: config.ClassicApiType{Api: "hosts-auto-update"},
},
},
Expand Down Expand Up @@ -982,7 +962,7 @@ func TestDownloadIntegrationOverwritesFolderAndManifestIfForced(t *testing.T) {
},
Group: "default",
Environment: projectName,
Template: contentOnlyTemplate{`{"custom-response": true, "name": "{{.name}}"}`},
Template: template.NewInMemoryTemplate("id", `{"custom-response": true, "name": "{{.name}}"}`),
Type: config.ClassicApiType{Api: "fake-id"},
},
},
Expand Down
4 changes: 2 additions & 2 deletions cmd/monaco/download/download_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func Test_checkForCircularDependencies(t *testing.T) {
"test_project": {
"dashboard": []config.Config{
{
Template: template.CreateTemplateFromString("some/path", "{}"),
Template: template.NewInMemoryTemplate("some/path", "{}"),
Parameters: map[string]parameter.Parameter{
"name": &valueParam.ValueParameter{Value: "name A"},
"ref": parameter.NewDummy(coordinate.Coordinate{Project: "test", Type: "dashboard", ConfigId: "b"}),
Expand All @@ -150,7 +150,7 @@ func Test_checkForCircularDependencies(t *testing.T) {
},
},
{
Template: template.CreateTemplateFromString("some/path", "{}"),
Template: template.NewInMemoryTemplate("some/path", "{}"),
Parameters: map[string]parameter.Parameter{
"name": &valueParam.ValueParameter{Value: "name A"},
"ref": parameter.NewDummy(coordinate.Coordinate{Project: "test", Type: "dashboard", ConfigId: "a"}),
Expand Down
5 changes: 3 additions & 2 deletions cmd/monaco/integrationtest/v2/references_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,11 @@ func findSetting(t *testing.T, confsPerType project.ConfigsPerType, api, name, p

for _, c := range confs {

content := c.Template.Content()
content, err := c.Template.Content()
assert.NoError(t, err)
// convert content to json
var jsonContent map[string]interface{}
err := json.Unmarshal([]byte(content), &jsonContent)
err = json.Unmarshal([]byte(content), &jsonContent)
assert.Nil(t, err, "failed to unmarshal content to json")

// get the setting name
Expand Down
6 changes: 4 additions & 2 deletions cmd/monaco/integrationtest/v2/settings_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,14 @@ func TestOldExternalIDGetsUpdated(t *testing.T) {
id, _ := idutils.GenerateExternalID(input)
return id, nil
}))
_, err := c.UpsertSettings(context.TODO(), dtclient.SettingsObject{
content, err := configToDeploy.Template.Content()
assert.NoError(t, err)
_, err = c.UpsertSettings(context.TODO(), dtclient.SettingsObject{
Coordinate: configToDeploy.Coordinate,
SchemaId: configToDeploy.Type.(config.SettingsType).SchemaId,
SchemaVersion: configToDeploy.Type.(config.SettingsType).SchemaVersion,
Scope: "environment",
Content: []byte(configToDeploy.Template.Content()),
Content: []byte(content),
OriginObjectId: configToDeploy.OriginObjectId,
}, dtclient.UpsertSettingsOptions{})
assert.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ func (c *Config) Render(properties map[string]interface{}) (string, error) {
return "", nil
}

templatePath := c.Template.Name()
if t, ok := c.Template.(template.FileBasedTemplate); ok {
var templatePath string // include path in errors if we know it
if t, ok := c.Template.(*template.FileBasedTemplate); ok {
templatePath = t.FilePath()
}
jskelin marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
2 changes: 1 addition & 1 deletion pkg/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func toParameterMap(params []parameter.NamedParameter) map[string]parameter.Para
func generateDummyTemplate(t *testing.T) template.Template {
newUUID, err := uuid.NewUUID()
assert.NoError(t, err)
templ := template.CreateTemplateFromString("deploy_test-"+newUUID.String(), "{}")
templ := template.NewInMemoryTemplate("deploy_test-"+newUUID.String(), "{}")
return templ
}

Expand Down
Loading
Loading