Merge pull request #848 from petrkotas/issue-838

Change template configuration to pointers
This commit is contained in:
Mangirdas Judeikis 2020-08-25 07:59:32 +01:00 коммит произвёл GitHub
Родитель 7f52c23580 18aed2ba88
Коммит 324be48961
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
6 изменённых файлов: 205 добавлений и 59 удалений

Просмотреть файл

@ -38,6 +38,11 @@ func deploy(ctx context.Context, log *logrus.Entry) error {
return err
}
err = config.CheckRequiredFields()
if err != nil {
return err
}
deployer, err := deployer.New(ctx, log, config, deployVersion, os.Getenv("FULL_DEPLOY") != "")
if err != nil {
return err

Просмотреть файл

@ -27,32 +27,32 @@ type RPConfig struct {
// Configuration represents configuration structure
type Configuration struct {
ACRResourceID string `json:"acrResourceId,omitempty"`
RPVersionStorageAccountName string `json:"rpVersionStorageAccountName,omitempty"`
ACRReplicaDisabled bool `json:"acrReplicaDisabled,omitempty"`
AdminAPICABundle string `json:"adminApiCaBundle,omitempty"`
AdminAPIClientCertCommonName string `json:"adminApiClientCertCommonName,omitempty"`
ClusterParentDomainName string `json:"clusterParentDomainName,omitempty"`
DatabaseAccountName string `json:"databaseAccountName,omitempty"`
ACRResourceID *string `json:"acrResourceId,omitempty"`
RPVersionStorageAccountName *string `json:"rpVersionStorageAccountName,omitempty"`
ACRReplicaDisabled *bool `json:"acrReplicaDisabled,omitempty"`
AdminAPICABundle *string `json:"adminApiCaBundle,omitempty"`
AdminAPIClientCertCommonName *string `json:"adminApiClientCertCommonName,omitempty"`
ClusterParentDomainName *string `json:"clusterParentDomainName,omitempty"`
DatabaseAccountName *string `json:"databaseAccountName,omitempty"`
ExtraClusterKeyvaultAccessPolicies []interface{} `json:"extraClusterKeyvaultAccessPolicies,omitempty"`
ExtraCosmosDBIPs []string `json:"extraCosmosDBIPs,omitempty"`
ExtraCosmosDBIPs []string `json:"extraCosmosDBIPs,omitempty" value:"required"`
ExtraServiceKeyvaultAccessPolicies []interface{} `json:"extraServiceKeyvaultAccessPolicies,omitempty"`
FPServerCertCommonName string `json:"fpServerCertCommonName,omitempty"`
FPServicePrincipalID string `json:"fpServicePrincipalId,omitempty"`
GlobalMonitoringKeyVaultURI string `json:"globalMonitoringKeyVaultUri,omitempty"`
GlobalResourceGroupName string `json:"globalResourceGroupName,omitempty"`
GlobalSubscriptionID string `json:"globalSubscriptionId,omitempty"`
KeyvaultPrefix string `json:"keyvaultPrefix,omitempty"`
MDMFrontendURL string `json:"mdmFrontendUrl,omitempty"`
MDSDConfigVersion string `json:"mdsdConfigVersion,omitempty"`
MDSDEnvironment string `json:"mdsdEnvironment,omitempty"`
RPImagePrefix string `json:"rpImagePrefix,omitempty"`
RPMode string `json:"rpMode,omitempty"`
FPServerCertCommonName *string `json:"fpServerCertCommonName,omitempty"`
FPServicePrincipalID *string `json:"fpServicePrincipalId,omitempty"`
GlobalMonitoringKeyVaultURI *string `json:"globalMonitoringKeyVaultUri,omitempty"`
GlobalResourceGroupName *string `json:"globalResourceGroupName,omitempty"`
GlobalSubscriptionID *string `json:"globalSubscriptionId,omitempty"`
KeyvaultPrefix *string `json:"keyvaultPrefix,omitempty"`
MDMFrontendURL *string `json:"mdmFrontendUrl,omitempty" value:"required"`
MDSDConfigVersion *string `json:"mdsdConfigVersion,omitempty"`
MDSDEnvironment *string `json:"mdsdEnvironment,omitempty"`
RPImagePrefix *string `json:"rpImagePrefix,omitempty"`
RPMode *string `json:"rpMode,omitempty"`
RPNSGSourceAddressPrefixes []string `json:"rpNsgSourceAddressPrefixes,omitempty"`
RPParentDomainName string `json:"rpParentDomainName,omitempty"`
SubscriptionResourceGroupName string `json:"subscriptionResourceGroupName,omitempty"`
SSHPublicKey string `json:"sshPublicKey,omitempty"`
VMSize string `json:"vmSize,omitempty"`
RPParentDomainName *string `json:"rpParentDomainName,omitempty"`
SubscriptionResourceGroupName *string `json:"subscriptionResourceGroupName,omitempty"`
SSHPublicKey *string `json:"sshPublicKey,omitempty"`
VMSize *string `json:"vmSize,omitempty"`
}
// GetConfig return RP configuration from the file
@ -97,3 +97,27 @@ func mergeConfig(primary, secondary *Configuration) (*Configuration, error) {
return primary, nil
}
// CheckRequiredFields validates configuration whether it provides required fields.
// Config is invalid if required fields are not provided.
func (conf *RPConfig) CheckRequiredFields() error {
configuration := conf.Configuration
v := reflect.ValueOf(*configuration)
missingFields := []string{}
for i := 0; i < v.NumField(); i++ {
required := v.Type().Field(i).Tag.Get("value") == "required"
if required && v.Field(i).IsZero() {
missingFields = append(missingFields, v.Type().Field(i).Name)
}
}
fmt.Println(missingFields)
if len(missingFields) == 0 {
return nil
}
return fmt.Errorf("Configuration has missing fields: %s", missingFields)
}

Просмотреть файл

@ -5,10 +5,13 @@ package deploy
import (
"encoding/json"
"fmt"
"reflect"
"strings"
"testing"
"github.com/Azure/go-autorest/autorest/to"
"github.com/Azure/ARO-RP/pkg/deploy/generator"
"github.com/Azure/ARO-RP/pkg/util/arm"
)
@ -51,6 +54,11 @@ func TestConfigurationFieldParity(t *testing.T) {
}
func TestMergeConfig(t *testing.T) {
databaseAccountName := to.StringPtr("databaseAccountName")
fpServerCertCommonName := to.StringPtr("fpServerCertCommonName")
fpServerSecondaryCommonName := to.StringPtr("fpServerSecondaryCommonName")
kvPrefix := to.StringPtr("keyvaultPrefix")
for _, tt := range []struct {
name string
primary Configuration
@ -63,17 +71,17 @@ func TestMergeConfig(t *testing.T) {
{
name: "overrides",
primary: Configuration{
DatabaseAccountName: "primary accountname",
FPServerCertCommonName: "primary fpcert",
DatabaseAccountName: databaseAccountName,
FPServerCertCommonName: fpServerCertCommonName,
},
secondary: Configuration{
FPServerCertCommonName: "secondary fpcert",
KeyvaultPrefix: "secondary kv",
FPServerCertCommonName: fpServerSecondaryCommonName,
KeyvaultPrefix: kvPrefix,
},
want: Configuration{
DatabaseAccountName: "primary accountname",
FPServerCertCommonName: "primary fpcert",
KeyvaultPrefix: "secondary kv",
DatabaseAccountName: databaseAccountName,
FPServerCertCommonName: fpServerCertCommonName,
KeyvaultPrefix: kvPrefix,
},
},
} {
@ -89,3 +97,60 @@ func TestMergeConfig(t *testing.T) {
})
}
}
func TestConfigNilable(t *testing.T) {
defer func() {
if r := recover(); r != nil {
t.Errorf("Configuration can contain only nilable types. %v", r)
}
}()
cfg := Configuration{}
val := reflect.ValueOf(cfg)
for i := 0; i < val.NumField(); i++ {
val.Field(i).IsNil()
}
}
func TestConfigRequiredValues(t *testing.T) {
AdminAPICABundle := "AdminAPICABundle"
ACRResourceID := "ACRResourceID"
ExtraCosmosDBIPs := "ExtraCosmosDBIPs"
MDMFrontendURL := "MDMFrontendURL"
for _, tt := range []struct {
name string
config RPConfig
expect error
}{
{
name: "valid config",
config: RPConfig{
Configuration: &Configuration{
ACRResourceID: &ACRResourceID,
AdminAPICABundle: &AdminAPICABundle,
ExtraCosmosDBIPs: []string{ExtraCosmosDBIPs},
MDMFrontendURL: &MDMFrontendURL,
},
},
expect: nil,
},
{
name: "invalid config",
config: RPConfig{
Configuration: &Configuration{
ACRResourceID: &ACRResourceID,
AdminAPICABundle: &AdminAPICABundle,
ExtraCosmosDBIPs: []string{ExtraCosmosDBIPs},
},
},
expect: fmt.Errorf("Configuration has missing fields: %s", "[MDMFrontendURL]"),
},
} {
valid := tt.config.CheckRequiredFields()
if valid != tt.expect && valid.Error() != tt.expect.Error() {
t.Errorf("Expected %s but got %s", tt.name, valid.Error())
}
}
}

Просмотреть файл

@ -71,8 +71,8 @@ func New(ctx context.Context, log *logrus.Entry, config *RPConfig, version strin
return &deployer{
log: log,
globaldeployments: features.NewDeploymentsClient(config.Configuration.GlobalSubscriptionID, authorizer),
globalrecordsets: dns.NewRecordSetsClient(config.Configuration.GlobalSubscriptionID, authorizer),
globaldeployments: features.NewDeploymentsClient(*config.Configuration.GlobalSubscriptionID, authorizer),
globalrecordsets: dns.NewRecordSetsClient(*config.Configuration.GlobalSubscriptionID, authorizer),
deployments: features.NewDeploymentsClient(config.SubscriptionID, authorizer),
groups: features.NewResourceGroupsClient(config.SubscriptionID, authorizer),
metricalerts: insights.NewMetricAlertsClient(config.SubscriptionID, authorizer),
@ -110,16 +110,16 @@ func (d *deployer) Deploy(ctx context.Context) error {
parameters := d.getParameters(template["parameters"].(map[string]interface{}))
parameters.Parameters["adminApiCaBundle"] = &arm.ParametersParameter{
Value: base64.StdEncoding.EncodeToString([]byte(d.config.Configuration.AdminAPICABundle)),
Value: base64.StdEncoding.EncodeToString([]byte(*d.config.Configuration.AdminAPICABundle)),
}
parameters.Parameters["domainName"] = &arm.ParametersParameter{
Value: d.config.Location + "." + d.config.Configuration.ClusterParentDomainName,
Value: d.config.Location + "." + *d.config.Configuration.ClusterParentDomainName,
}
parameters.Parameters["extraCosmosDBIPs"] = &arm.ParametersParameter{
Value: strings.Join(d.config.Configuration.ExtraCosmosDBIPs, ","),
}
parameters.Parameters["rpImage"] = &arm.ParametersParameter{
Value: d.config.Configuration.RPImagePrefix + ":" + d.version,
Value: *d.config.Configuration.RPImagePrefix + ":" + d.version,
}
parameters.Parameters["rpServicePrincipalId"] = &arm.ParametersParameter{
Value: msi.PrincipalID.String(),
@ -177,12 +177,12 @@ func (d *deployer) configureDNS(ctx context.Context) error {
return err
}
zone, err := d.zones.Get(ctx, d.config.ResourceGroupName, d.config.Location+"."+d.config.Configuration.ClusterParentDomainName)
zone, err := d.zones.Get(ctx, d.config.ResourceGroupName, d.config.Location+"."+*d.config.Configuration.ClusterParentDomainName)
if err != nil {
return err
}
_, err = d.globalrecordsets.CreateOrUpdate(ctx, d.config.Configuration.GlobalResourceGroupName, d.config.Configuration.RPParentDomainName, "rp."+d.config.Location, mgmtdns.A, mgmtdns.RecordSet{
_, err = d.globalrecordsets.CreateOrUpdate(ctx, *d.config.Configuration.GlobalResourceGroupName, *d.config.Configuration.RPParentDomainName, "rp."+d.config.Location, mgmtdns.A, mgmtdns.RecordSet{
RecordSetProperties: &mgmtdns.RecordSetProperties{
TTL: to.Int64Ptr(3600),
ARecords: &[]mgmtdns.ARecord{
@ -203,7 +203,7 @@ func (d *deployer) configureDNS(ctx context.Context) error {
})
}
_, err = d.globalrecordsets.CreateOrUpdate(ctx, d.config.Configuration.GlobalResourceGroupName, d.config.Configuration.ClusterParentDomainName, d.config.Location, mgmtdns.NS, mgmtdns.RecordSet{
_, err = d.globalrecordsets.CreateOrUpdate(ctx, *d.config.Configuration.GlobalResourceGroupName, *d.config.Configuration.ClusterParentDomainName, d.config.Location, mgmtdns.NS, mgmtdns.RecordSet{
RecordSetProperties: &mgmtdns.RecordSetProperties{
TTL: to.Int64Ptr(3600),
NsRecords: &nsRecords,
@ -242,9 +242,12 @@ func (d *deployer) removeOldMetricAlerts(ctx context.Context) error {
// from d.config.Configuration.
func (d *deployer) getParameters(ps map[string]interface{}) *arm.Parameters {
m := map[string]interface{}{}
v := reflect.ValueOf(*d.config.Configuration)
for i := 0; i < v.NumField(); i++ {
if v.Field(i).IsNil() {
continue
}
m[strings.SplitN(v.Type().Field(i).Tag.Get("json"), ",", 2)[0]] = v.Field(i).Interface()
}
@ -253,8 +256,15 @@ func (d *deployer) getParameters(ps map[string]interface{}) *arm.Parameters {
}
for p := range ps {
// do not convert empty fields
// makes default values templates work
v, ok := m[p]
if !ok {
continue
}
parameters.Parameters[p] = &arm.ParametersParameter{
Value: m[p],
Value: v,
}
}

Просмотреть файл

@ -7,10 +7,15 @@ import (
"reflect"
"testing"
"github.com/Azure/go-autorest/autorest/to"
"github.com/Azure/ARO-RP/pkg/util/arm"
)
func TestGetParameters(t *testing.T) {
databaseAccountName := to.StringPtr("databaseAccountName")
adminApiCaBundle := to.StringPtr("adminApiCaBundle")
extraClusterKeyVaultAccessPolicies := []interface{}{"a", "b", 1}
for _, tt := range []struct {
name string
ps map[string]interface{}
@ -18,7 +23,7 @@ func TestGetParameters(t *testing.T) {
want arm.Parameters
}{
{
name: "no parameters",
name: "when no parameters are present only default is returned",
want: arm.Parameters{
Parameters: map[string]*arm.ParametersParameter{
"fullDeploy": {
@ -28,26 +33,27 @@ func TestGetParameters(t *testing.T) {
},
},
{
name: "valid",
name: "when all parameters present, everything is copied",
ps: map[string]interface{}{
"adminApiCaBundle": nil,
"databaseAccountName": nil,
"extraClusterKeyvaultAccessPolicies": nil,
},
config: Configuration{
DatabaseAccountName: "databaseAccountName",
ExtraClusterKeyvaultAccessPolicies: []interface{}{"a", 1},
DatabaseAccountName: databaseAccountName,
AdminAPICABundle: adminApiCaBundle,
ExtraClusterKeyvaultAccessPolicies: extraClusterKeyVaultAccessPolicies,
},
want: arm.Parameters{
Parameters: map[string]*arm.ParametersParameter{
"adminApiCaBundle": {
Value: "",
},
"databaseAccountName": {
Value: "databaseAccountName",
Value: databaseAccountName,
},
"extraClusterKeyvaultAccessPolicies": {
Value: []interface{}{"a", 1},
Value: extraClusterKeyVaultAccessPolicies,
},
"adminApiCaBundle": {
Value: adminApiCaBundle,
},
"fullDeploy": {
Value: false,
@ -56,16 +62,50 @@ func TestGetParameters(t *testing.T) {
},
},
{
name: "nil slice",
name: "when parameters with nil config are present, they are not returned",
ps: map[string]interface{}{
"adminApiCaBundle": nil,
"databaseAccountName": nil,
"extraClusterKeyvaultAccessPolicies": nil,
},
config: Configuration{
DatabaseAccountName: databaseAccountName,
},
want: arm.Parameters{
Parameters: map[string]*arm.ParametersParameter{
"databaseAccountName": {
Value: databaseAccountName,
},
"fullDeploy": {
Value: false,
},
},
},
},
{
name: "when nil slice parameter is present it is skipped",
ps: map[string]interface{}{
"extraClusterKeyvaultAccessPolicies": nil,
},
config: Configuration{},
want: arm.Parameters{
Parameters: map[string]*arm.ParametersParameter{
"extraClusterKeyvaultAccessPolicies": {
Value: []interface{}(nil),
"fullDeploy": {
Value: false,
},
},
},
},
{
name: "when malformed paramater is present, it is skipped",
ps: map[string]interface{}{
"dutabaseAccountName": nil,
},
config: Configuration{
DatabaseAccountName: databaseAccountName,
},
want: arm.Parameters{
Parameters: map[string]*arm.ParametersParameter{
"fullDeploy": {
Value: false,
},
@ -79,9 +119,11 @@ func TestGetParameters(t *testing.T) {
}
got := d.getParameters(tt.ps)
if !reflect.DeepEqual(got, &tt.want) {
t.Errorf("%#v", got)
}
})
}
}

Просмотреть файл

@ -29,14 +29,14 @@ func (d *deployer) PreDeploy(ctx context.Context) error {
}
if d.fullDeploy {
_, err = d.groups.CreateOrUpdate(ctx, d.config.Configuration.SubscriptionResourceGroupName, mgmtfeatures.ResourceGroup{
_, err = d.groups.CreateOrUpdate(ctx, *d.config.Configuration.SubscriptionResourceGroupName, mgmtfeatures.ResourceGroup{
Location: to.StringPtr("centralus"),
})
if err != nil {
return err
}
_, err = d.groups.CreateOrUpdate(ctx, d.config.Configuration.GlobalResourceGroupName, mgmtfeatures.ResourceGroup{
_, err = d.groups.CreateOrUpdate(ctx, *d.config.Configuration.GlobalResourceGroupName, mgmtfeatures.ResourceGroup{
Location: to.StringPtr("centralus"),
})
if err != nil {
@ -69,7 +69,7 @@ func (d *deployer) PreDeploy(ctx context.Context) error {
// Due to https://github.com/Azure/azure-resource-manager-schemas/issues/1067
// we can't use conditions to define ACR replication object deployment.
// We use ACRReplicaDisabled in the same way as we use fullDeploy.
if !d.config.Configuration.ACRReplicaDisabled {
if d.config.Configuration.ACRReplicaDisabled != nil && !*d.config.Configuration.ACRReplicaDisabled {
err = d.deployGloalACRReplication(ctx)
if err != nil {
return err
@ -117,7 +117,7 @@ func (d *deployer) deployGlobal(ctx context.Context, rpServicePrincipalID string
}
d.log.Infof("deploying %s", deploymentName)
return d.globaldeployments.CreateOrUpdateAndWait(ctx, d.config.Configuration.GlobalResourceGroupName, deploymentName, mgmtfeatures.Deployment{
return d.globaldeployments.CreateOrUpdateAndWait(ctx, *d.config.Configuration.GlobalResourceGroupName, deploymentName, mgmtfeatures.Deployment{
Properties: &mgmtfeatures.DeploymentProperties{
Template: template,
Mode: mgmtfeatures.Incremental,
@ -146,7 +146,7 @@ func (d *deployer) deployGloalACRReplication(ctx context.Context) error {
}
d.log.Infof("deploying %s", deploymentName)
return d.globaldeployments.CreateOrUpdateAndWait(ctx, d.config.Configuration.GlobalResourceGroupName, deploymentName, mgmtfeatures.Deployment{
return d.globaldeployments.CreateOrUpdateAndWait(ctx, *d.config.Configuration.GlobalResourceGroupName, deploymentName, mgmtfeatures.Deployment{
Properties: &mgmtfeatures.DeploymentProperties{
Template: template,
Mode: mgmtfeatures.Incremental,
@ -199,7 +199,7 @@ func (d *deployer) deploySubscription(ctx context.Context) error {
parameters := d.getParameters(template["parameters"].(map[string]interface{}))
d.log.Infof("deploying %s", deploymentName)
return d.deployments.CreateOrUpdateAndWait(ctx, d.config.Configuration.SubscriptionResourceGroupName, deploymentName, mgmtfeatures.Deployment{
return d.deployments.CreateOrUpdateAndWait(ctx, *d.config.Configuration.SubscriptionResourceGroupName, deploymentName, mgmtfeatures.Deployment{
Properties: &mgmtfeatures.DeploymentProperties{
Template: template,
Mode: mgmtfeatures.Incremental,
@ -277,7 +277,7 @@ func (d *deployer) deployPreDeploy(ctx context.Context, rpServicePrincipalID str
}
func (d *deployer) configureServiceSecrets(ctx context.Context) error {
serviceKeyVaultURI := "https://" + d.config.Configuration.KeyvaultPrefix + "-svc.vault.azure.net/"
serviceKeyVaultURI := "https://" + *d.config.Configuration.KeyvaultPrefix + "-svc.vault.azure.net/"
secrets, err := d.keyvault.GetSecrets(ctx, serviceKeyVaultURI, nil)
if err != nil {
return err