Skip to content
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
2 changes: 2 additions & 0 deletions cli/azd/.vscode/cspell.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ words:
- runcontext
- unmarshals
- usgovcloudapi
- jdbc
- bicept
languageSettings:
- languageId: go
ignoreRegExpList:
Expand Down
8 changes: 7 additions & 1 deletion cli/azd/internal/appdetect/java.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,14 @@ func detectDependencies(mavenProject *mavenProject, project *Project) (*Project,
databaseDepMap[DbMySql] = struct{}{}
}

if dep.GroupId == "org.postgresql" && dep.ArtifactId == "postgresql" {
if (dep.GroupId == "org.postgresql" && dep.ArtifactId == "postgresql") ||
(dep.GroupId == "com.azure.spring" && dep.ArtifactId == "spring-cloud-azure-starter-jdbc-postgresql") {
databaseDepMap[DbPostgres] = struct{}{}
// todo: Only call this func when it's a spring-boot project
err := addPostgresqlConnectionProperties(project.Path)
if err != nil {
return nil, err
}
}
}

Expand Down
193 changes: 193 additions & 0 deletions cli/azd/internal/appdetect/spring_boot_properties_file.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
package appdetect

import (
"bufio"
"fmt"
"os"
"path/filepath"
"strings"
)

type property struct {
key string
value string
}
type propertyMergeFunc func(string, string) string

const azureProfileName = "azure"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if in the user's application already have one azure profile?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.
In this case, original value of spring.profiles.active will not be changed. See these test cases:

image

And here is related source code:

image


// todo: manage following placeholders and together with resources.bicept.

const placeholderPostgresHost = "${POSTGRES_HOST}"
const placeholderPostgresPort = "${POSTGRES_PORT}"
const placeholderPostgresDatabase = "${POSTGRES_DATABASE}"
const placeholderPostgresUsername = "${POSTGRES_USERNAME}"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the conventional env names that defined in azd.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean these should be defined in a common place, instead of being defined in java

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Updated in this commit: cdc9acd


// Split to fix this problem: "G101: Potential hardcoded credentials (gosec)"
const placeholderPostgresPassword = "${POSTGRES_PASS" + "WORD}"
const placeholderPostgresJdbcUrl = "jdbc:postgresql://" + placeholderPostgresHost + ":" + placeholderPostgresPort +
"/" + placeholderPostgresDatabase

var postgresqlProperties = []property{
{"spring.datasource.url", placeholderPostgresJdbcUrl},
{"spring.datasource.username", placeholderPostgresUsername},
{"spring.datasource.password", placeholderPostgresPassword},
}

var applicationPropertiesRelativePath = filepath.Join("src", "main", "resources",
"application.properties")
var applicationAzurePropertiesRelativePath = filepath.Join("src", "main", "resources",
"application-"+azureProfileName+".properties")

// todo: support other file suffix. Example: application.yml, application.yaml
func addPostgresqlConnectionProperties(projectPath string) error {
err := addPostgresqlConnectionPropertiesIntoPropertyFile(projectPath)
if err != nil {
return err
}
return activeAzureProfile(projectPath)
}

func addPostgresqlConnectionPropertiesIntoPropertyFile(projectPath string) error {
filePath := filepath.Join(projectPath, applicationAzurePropertiesRelativePath)
return updatePropertyFile(filePath, postgresqlProperties, keepNewValue)
}

func keepNewValue(_ string, newValue string) string {
return newValue
}

func activeAzureProfile(projectPath string) error {
filePath := filepath.Join(projectPath, applicationPropertiesRelativePath)
var newProperties = []property{
{"spring.profiles.active", azureProfileName},
}
return updatePropertyFile(filePath, newProperties, appendToCommaSeparatedValues)
}

func appendToCommaSeparatedValues(commaSeparatedValues string, newValue string) string {
if commaSeparatedValues == "" {
return newValue
}
var values []string
for _, value := range strings.SplitN(commaSeparatedValues, ",", -1) {
value = strings.TrimSpace(value)
if value != "" {
values = append(values, value)
}
}
if !contains(values, azureProfileName) {
values = append(values, azureProfileName)
}
return strings.Join(values, ",")
}

func contains(a []string, x string) bool {
for _, n := range a {
if x == n {
return true
}
}
return false
}

func updatePropertyFile(filePath string, newProperties []property, function propertyMergeFunc) error {
err := createFileIfNotExist(filePath)
if err != nil {
return err
}
properties, err := readProperties(filePath)
if err != nil {
return err
}
properties = updateProperties(properties, newProperties, function)
err = writeProperties(filePath, properties)
if err != nil {
return err
}
return nil
}

func createFileIfNotExist(filePath string) error {
dir := filepath.Dir(filePath)
if _, err := os.Stat(dir); os.IsNotExist(err) {
if err := os.MkdirAll(dir, os.ModePerm); err != nil {
return fmt.Errorf("failed to create directory: %w", err)
}
}
if _, err := os.Stat(filePath); os.IsNotExist(err) {
file, err := os.Create(filePath)
if err != nil {
return fmt.Errorf("failed to create file: %w", err)
}
defer file.Close()
}
return nil
}

func readProperties(filePath string) ([]property, error) {
file, err := os.Open(filePath)
if err != nil {
return nil, err
}
defer file.Close()

var properties []property
scanner := bufio.NewScanner(file)
for scanner.Scan() {
line := scanner.Text()
if strings.TrimSpace(line) == "" || strings.HasPrefix(line, "#") {
continue
}
parts := strings.SplitN(line, "=", 2)
if len(parts) == 2 {
key := strings.TrimSpace(parts[0])
value := strings.TrimSpace(parts[1])
properties = append(properties, property{key, value})
}
}

if err := scanner.Err(); err != nil {
return nil, err
}

return properties, nil
}

func updateProperties(properties []property,
newProperties []property, function propertyMergeFunc) []property {
for _, newProperty := range newProperties {
if index := getKeyIndex(properties, newProperty.key); index != -1 {
properties[index].value = function(properties[index].value, newProperty.value)
} else {
properties = append(properties, newProperty)
}
}
return properties
}

func getKeyIndex(properties []property, key string) int {
for i, prop := range properties {
if prop.key == key {
return i
}
}
return -1
}

func writeProperties(filePath string, properties []property) error {
file, err := os.Create(filePath)
if err != nil {
return err
}
defer file.Close()

writer := bufio.NewWriter(file)
for _, p := range properties {
_, err := writer.WriteString(fmt.Sprintf("%s=%s\n", p.key, p.value))
if err != nil {
return err
}
}
return writer.Flush()
}
141 changes: 141 additions & 0 deletions cli/azd/internal/appdetect/spring_boot_properties_file_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
package appdetect

import (
"os"
"path/filepath"
"reflect"
"testing"

"github.com/stretchr/testify/assert"
)

const postgresPropertiesOriginalContent = `spring.datasource.url=jdbc:postgresql://localhost:5432/dbname?sslmode=require
spring.datasource.username=admin
spring.datasource.password=secret
`

const postgresPropertiesUpdatedContent = `spring.datasource.url=` + placeholderPostgresJdbcUrl + `
spring.datasource.username=` + placeholderPostgresUsername + `
spring.datasource.password=` + placeholderPostgresPassword + `
`

var postgresPropertiesOriginalMap = []property{
{"spring.datasource.url", "jdbc:postgresql://localhost:5432/dbname?sslmode=require"},
{"spring.datasource.username", "admin"},
{"spring.datasource.password", "secret"},
}

func TestAddPostgresqlConnectionProperties(t *testing.T) {
tests := []struct {
name string
inputApplicationPropertiesContent string
inputApplicationAzurePropertiesContent string
outputApplicationPropertiesContent string
outputApplicationAzurePropertiesContent string
}{
{
name: "no content",
inputApplicationPropertiesContent: "",
inputApplicationAzurePropertiesContent: "",
outputApplicationPropertiesContent: "spring.profiles.active=" + azureProfileName + "\n",
outputApplicationAzurePropertiesContent: postgresPropertiesUpdatedContent,
},
{
name: "override original content",
inputApplicationPropertiesContent: "spring.profiles.active=" + azureProfileName,
inputApplicationAzurePropertiesContent: postgresPropertiesOriginalContent,
outputApplicationPropertiesContent: "spring.profiles.active=" + azureProfileName + "\n",
outputApplicationAzurePropertiesContent: postgresPropertiesUpdatedContent,
},
{
name: "append original content",
inputApplicationPropertiesContent: "aaa=xxx\n" + "spring.profiles.active=production , cloud,,",
inputApplicationAzurePropertiesContent: "bbb=yyy\n" + postgresPropertiesOriginalContent,
outputApplicationPropertiesContent: "aaa=xxx\n" + "spring.profiles.active=production,cloud," +
azureProfileName + "\n",
outputApplicationAzurePropertiesContent: "bbb=yyy\n" + postgresPropertiesUpdatedContent,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tempDir := t.TempDir()
applicationPropertiesPath := filepath.Join(tempDir, applicationPropertiesRelativePath)
createFileIfContentIsNotEmpty(t, applicationPropertiesPath, tt.inputApplicationPropertiesContent)
applicationAzurePropertiesPath := filepath.Join(tempDir, applicationAzurePropertiesRelativePath)
createFileIfContentIsNotEmpty(t, applicationAzurePropertiesPath, tt.inputApplicationAzurePropertiesContent)
err := addPostgresqlConnectionProperties(tempDir)
assert.NoError(t, err)
assertFileContent(t, applicationPropertiesPath, tt.outputApplicationPropertiesContent)
assertFileContent(t, applicationAzurePropertiesPath, tt.outputApplicationAzurePropertiesContent)
})
}
}

func createFileIfContentIsNotEmpty(t *testing.T, path string, content string) {
if content == "" {
return
}

dir := filepath.Dir(path)
if _, err := os.Stat(dir); os.IsNotExist(err) {
err := os.MkdirAll(dir, os.ModePerm)
assert.NoError(t, err)
}
file, err := os.Create(path)
assert.NoError(t, err)
defer file.Close()
err = os.WriteFile(path, []byte(content), 0600)
assert.NoError(t, err)
}

func assertFileContent(t *testing.T, path string, content string) {
actualContent, err := os.ReadFile(path)
assert.NoError(t, err)
assert.Equal(t, content, string(actualContent))
}

func TestReadProperties(t *testing.T) {
tempFile, err := os.CreateTemp("", "test.properties")
if err != nil {
t.Fatal(err)
}
defer os.Remove(tempFile.Name())

if _, err := tempFile.Write([]byte(postgresPropertiesOriginalContent)); err != nil {
t.Fatal(err)
}
if err := tempFile.Close(); err != nil {
t.Fatal(err)
}

properties, err := readProperties(tempFile.Name())
if err != nil {
t.Fatalf("readProperties() error = %v", err)
}

if !reflect.DeepEqual(properties, postgresPropertiesOriginalMap) {
t.Errorf("readProperties() = %v, want %v", properties, postgresPropertiesOriginalMap)
}
}

func TestWriteProperties(t *testing.T) {
tempFile, err := os.CreateTemp("", "test.properties")
if err != nil {
t.Fatal(err)
}
defer os.Remove(tempFile.Name())

if err := writeProperties(tempFile.Name(), postgresPropertiesOriginalMap); err != nil {
t.Fatalf("writeProperties() error = %v", err)
}

content, err := os.ReadFile(tempFile.Name())
if err != nil {
t.Fatal(err)
}

if string(content) != postgresPropertiesOriginalContent {
t.Errorf("writeProperties() = %v, want %v", string(content), postgresPropertiesOriginalContent)
}
}
6 changes: 3 additions & 3 deletions cli/azd/resources/scaffold/templates/resources.bicept
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ module {{bicepName .Name}}FetchLatestImage './modules/fetch-container-image.bice
name: '{{bicepName .Name}}-fetch-image'
params: {
exists: {{bicepName .Name}}Exists
name: '{{.Name}}'
name: '{{containerAppName .Name}}'
}
}

Expand All @@ -217,7 +217,7 @@ var {{bicepName .Name}}Env = map(filter({{bicepName .Name}}AppSettingsArray, i =
module {{bicepName .Name}} 'br/public:avm/res/app/container-app:0.8.0' = {
name: '{{bicepName .Name}}'
params: {
name: '{{.Name}}'
name: '{{containerAppName .Name}}'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious about the change here -- we did intentionally move away from containerAppName .Name to .Name in #4434

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we did intentionally move away from containerAppName .Name to .Name in #4434

  1. Do you mean change from .Name to containerAppName .Name in easy-init: allow for longer container app names #4434 ?

  2. In think the mistake (missing containerAppName) is added in this PR: https://github.com/Azure/azure-dev/pull/4455/files

image

image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I think when #4455 was made, we also started adding upfront name validation in azd add (UI layer, not at the schema layer).

Keeping it as .Name instead of containerAppName .Name (which does the truncation) transformation may help with #4653, so I think .Name still looks better directionally. What do you think?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about doing like this:

  1. In current PR, keep using containerAppName .Name because it can solve the problem appeared in the sample project for current PR: https://github.com/azure-javaee/azure-spring-boot-samples/blob/788cc9a14f3fa2bfa826b0e875ba6dd729726dfa/postgresql/spring-cloud-azure-starter-jdbc-postgresql/spring-cloud-azure-postgresql-sample/pom.xml#L39
  2. For [Issue] Failed to deploy app when app name too long or different app name has same long prefix #4653, we can fix it in another PR. In that PR, the same name prefix should be considered,

{{- if ne .Port 0}}
ingressTargetPort: {{.Port}}
{{- end}}
Expand Down Expand Up @@ -353,7 +353,7 @@ module {{bicepName .Name}} 'br/public:avm/res/app/container-app:0.8.0' = {
{{- range $i, $e := .Frontend.Backends}}
{
name: '{{upper .Name}}_BASE_URL'
value: 'https://{{.Name}}.${containerAppsEnvironment.outputs.defaultDomain}'
value: 'https://{{containerAppName .Name}}.${containerAppsEnvironment.outputs.defaultDomain}'
}
{{- end}}
{{- end}}
Expand Down