Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

fix(#38): add sa_token_encrypted to cluster model #40

Merged
merged 4 commits into from
Dec 5, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
4 changes: 3 additions & 1 deletion cluster/repository/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ type Cluster struct {
LoggingURL string
// Application host name used by the cluster
AppDNS string
// Encrypted Service Account token
// Service Account token
SaToken string
// Service Account name
SaUsername string
// SA Token encrypted
SaTokenEncrypted bool
// Token Provider ID
TokenProviderID string
// OAuthClient ID used to link users account
Expand Down
1 change: 1 addition & 0 deletions cluster/service/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func (c clusterService) CreateOrSaveClusterFromConfig(ctx context.Context) error

SaToken: configCluster.ServiceAccountToken,
SaUsername: configCluster.ServiceAccountUsername,
SaTokenEncrypted: *configCluster.ServiceAccountTokenEncrypted,
TokenProviderID: configCluster.TokenProviderID,
AuthClientID: configCluster.AuthClientID,
AuthClientSecret: configCluster.AuthClientSecret,
Expand Down
2 changes: 2 additions & 0 deletions configuration/conf-files/oso-clusters.conf
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"app-dns":"b542.starter-us-east-1a.openshiftapps.com",
"service-account-token":"sdfjdlfjdfkjdlfjd12324434543085djdfjd084508gfdkjdofkjg43854085dlkjdlk",
"service-account-username":"dsaas",
"service-account-token-encrypted": false,
"token-provider-id":"886c7ea3-ef97-443d-b345-de94b94bb65d",
"auth-client-id":"autheast1a",
"auth-client-secret":"autheast1asecret",
Expand All @@ -41,6 +42,7 @@
"app-dns":"b542.starter-us-east-3a.openshiftapps.com",
"service-account-token":"fkdjhfdsjfgfdjlsflhjgsafgskfdsagrwgwerwshbdjasbdjbsahdbsagbdyhsbdesbh",
"service-account-username":"dsaas",
"service-account-token-encrypted": true,
"token-provider-id":"1c09073a-13ad-4add-b0ff-197eaf18fc37",
"auth-client-id":"autheast3a",
"auth-client-secret":"autheast3asecret",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"app-dns":"b542.starter-us-east-1a.openshiftapps.com",
"service-account-token":"sdfjdlfjdfkjdlfjd12324434543085djdfjd084508gfdkjdofkjg43854085dlkjdlk",
"service-account-username":"dsaas",
"service-account-token-encrypted": false,
"token-provider-id":"886c7ea3-ef97-443d-b345-de94b94bb65d",
"auth-client-id":"autheast1a",
"auth-client-secret":"autheast1asecret",
Expand All @@ -41,6 +42,7 @@
"app-dns":"b542.starter-us-east-3a.openshiftapps.com",
"service-account-token":"fkdjhfdsjfgfdjlsflhjgsafgskfdsagrwgwerwshbdjasbdjbsahdbsagbdyhsbdesbh",
"service-account-username":"dsaas",
"service-account-token-encrypted": true,
"token-provider-id":"1c09073a-13ad-4add-b0ff-197eaf18fc37",
"auth-client-id":"autheast3a",
"auth-client-secret":"autheast3asecret",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"app-dns":"8a09.starter-us-east-2.openshiftapps.com",
"service-account-token":"fX0nH3d68LQ6SK5wBE6QeKJ6X8AZGVQO3dGQZZETakhmgmWAqr2KDFXE65KUwBO69aWoq",
"service-account-username":"dsaas",
"service-account-token-encrypted": false,
"token-provider-id":"f867ac10-5e05-4359-a0c6-b855ece59090",
"auth-client-id":"autheast2",
"auth-client-secret":"autheast2secret",
Expand Down
41 changes: 26 additions & 15 deletions configuration/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,20 +77,21 @@ type clusterConfig struct {

// Cluster represents a cluster from configuration
type Cluster struct {
Name string `mapstructure:"name"`
APIURL string `mapstructure:"api-url"`
ConsoleURL string `mapstructure:"console-url"` // Optional in oso-clusters.conf
MetricsURL string `mapstructure:"metrics-url"` // Optional in oso-clusters.conf
LoggingURL string `mapstructure:"logging-url"` // Optional in oso-clusters.conf
AppDNS string `mapstructure:"app-dns"`
ServiceAccountToken string `mapstructure:"service-account-token"`
ServiceAccountUsername string `mapstructure:"service-account-username"`
TokenProviderID string `mapstructure:"token-provider-id"`
AuthClientID string `mapstructure:"auth-client-id"`
AuthClientSecret string `mapstructure:"auth-client-secret"`
AuthClientDefaultScope string `mapstructure:"auth-client-default-scope"`
Type string `mapstructure:"type"` // Optional in oso-clusters.conf ('OSO' by default)
CapacityExhausted bool `mapstructure:"capacity-exhausted"` // Optional in oso-clusters.conf ('false' by default)
Name string `mapstructure:"name"`
APIURL string `mapstructure:"api-url"`
ConsoleURL string `mapstructure:"console-url"` // Optional in oso-clusters.conf
MetricsURL string `mapstructure:"metrics-url"` // Optional in oso-clusters.conf
LoggingURL string `mapstructure:"logging-url"` // Optional in oso-clusters.conf
AppDNS string `mapstructure:"app-dns"`
ServiceAccountToken string `mapstructure:"service-account-token"`
ServiceAccountUsername string `mapstructure:"service-account-username"`
ServiceAccountTokenEncrypted *bool `mapstructure:"service-account-token-encrypted"` // Optional in oso-clusters.conf ('true' by default)
TokenProviderID string `mapstructure:"token-provider-id"`
AuthClientID string `mapstructure:"auth-client-id"`
AuthClientSecret string `mapstructure:"auth-client-secret"`
AuthClientDefaultScope string `mapstructure:"auth-client-default-scope"`
Type string `mapstructure:"type"` // Optional in oso-clusters.conf ('OSO' by default)
CapacityExhausted bool `mapstructure:"capacity-exhausted"` // Optional in oso-clusters.conf ('false' by default)
}

// ConfigurationData encapsulates the Viper configuration object which stores the configuration data in-memory.
Expand Down Expand Up @@ -209,6 +210,11 @@ func (c *ConfigurationData) initClusterConfig(clusterConfigFile, defaultClusterC
if configCluster.Type == "" {
configCluster.Type = cluster.OSO
}

if configCluster.ServiceAccountTokenEncrypted == nil {
configCluster.ServiceAccountTokenEncrypted = IsSaTokenEncrypted(true)
}

c.clusters[configCluster.APIURL] = configCluster
}

Expand Down Expand Up @@ -236,7 +242,7 @@ func (c *ConfigurationData) checkClusterConfig() error {
err = errors.Errorf("%s; key %v is missing in cluster config", err.Error(), tag)
ok = false
}
case bool:
case bool, *bool:
// Ignore
default:
err = errors.Errorf("%s; wrong type of key %v", err.Error(), tag)
Expand Down Expand Up @@ -613,3 +619,8 @@ func (c *ConfigurationData) GetDevModePrivateKey() []byte {
}
return nil
}

// IsSaTokenEncrypted return pointer to bool
func IsSaTokenEncrypted(encrypted bool) *bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

the name of this function is confusing: you just return a pointer to the given bool, don't you?

Copy link
Contributor Author

@dipak-pawar dipak-pawar Dec 5, 2018

Choose a reason for hiding this comment

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

ok, let me change that to PointerToBool?

return &encrypted
}
125 changes: 65 additions & 60 deletions configuration/configuration_blackbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,18 +192,19 @@ func TestClusterConfigurationWithGeneratedURLs(t *testing.T) {
clusterConfig, err := configuration.NewConfigurationData("", "./conf-files/tests/oso-clusters-custom-urls.conf")
require.Nil(t, err)
checkCluster(t, clusterConfig.GetClusters(), configuration.Cluster{
Name: "us-east-2",
APIURL: "https://api.starter-us-east-2.openshift.com",
ConsoleURL: "custom.console.url",
MetricsURL: "custom.metrics.url",
LoggingURL: "custom.logging.url",
AppDNS: "8a09.starter-us-east-2.openshiftapps.com",
ServiceAccountToken: "fX0nH3d68LQ6SK5wBE6QeKJ6X8AZGVQO3dGQZZETakhmgmWAqr2KDFXE65KUwBO69aWoq",
ServiceAccountUsername: "dsaas",
TokenProviderID: "f867ac10-5e05-4359-a0c6-b855ece59090",
AuthClientID: "autheast2",
AuthClientSecret: "autheast2secret",
AuthClientDefaultScope: "user:full",
Name: "us-east-2",
APIURL: "https://api.starter-us-east-2.openshift.com",
ConsoleURL: "custom.console.url",
MetricsURL: "custom.metrics.url",
LoggingURL: "custom.logging.url",
AppDNS: "8a09.starter-us-east-2.openshiftapps.com",
ServiceAccountToken: "fX0nH3d68LQ6SK5wBE6QeKJ6X8AZGVQO3dGQZZETakhmgmWAqr2KDFXE65KUwBO69aWoq",
ServiceAccountUsername: "dsaas",
ServiceAccountTokenEncrypted: configuration.IsSaTokenEncrypted(false),
TokenProviderID: "f867ac10-5e05-4359-a0c6-b855ece59090",
AuthClientID: "autheast2",
AuthClientSecret: "autheast2secret",
AuthClientDefaultScope: "user:full",
Type: cluster.OSO,
})
}
Expand All @@ -230,66 +231,70 @@ func TestClusterConfigurationFromInvalidFile(t *testing.T) {

func checkClusterConfiguration(t *testing.T, clusters map[string]configuration.Cluster) {
checkCluster(t, clusters, configuration.Cluster{
Name: "us-east-2",
APIURL: "https://api.starter-us-east-2.openshift.com",
ConsoleURL: "https://console.starter-us-east-2.openshift.com/console",
MetricsURL: "https://metrics.starter-us-east-2.openshift.com",
LoggingURL: "https://console.starter-us-east-2.openshift.com/console",
AppDNS: "8a09.starter-us-east-2.openshiftapps.com",
ServiceAccountToken: "fX0nH3d68LQ6SK5wBE6QeKJ6X8AZGVQO3dGQZZETakhmgmWAqr2KDFXE65KUwBO69aWoq",
ServiceAccountUsername: "dsaas",
TokenProviderID: "f867ac10-5e05-4359-a0c6-b855ece59090",
AuthClientID: "autheast2",
AuthClientSecret: "autheast2secret",
AuthClientDefaultScope: "user:full",
Name: "us-east-2",
APIURL: "https://api.starter-us-east-2.openshift.com",
ConsoleURL: "https://console.starter-us-east-2.openshift.com/console",
MetricsURL: "https://metrics.starter-us-east-2.openshift.com",
LoggingURL: "https://console.starter-us-east-2.openshift.com/console",
AppDNS: "8a09.starter-us-east-2.openshiftapps.com",
ServiceAccountToken: "fX0nH3d68LQ6SK5wBE6QeKJ6X8AZGVQO3dGQZZETakhmgmWAqr2KDFXE65KUwBO69aWoq",
ServiceAccountUsername: "dsaas",
ServiceAccountTokenEncrypted: configuration.IsSaTokenEncrypted(true),
TokenProviderID: "f867ac10-5e05-4359-a0c6-b855ece59090",
AuthClientID: "autheast2",
AuthClientSecret: "autheast2secret",
AuthClientDefaultScope: "user:full",
Type: "OSO",
CapacityExhausted: false,
})
checkCluster(t, clusters, configuration.Cluster{
Name: "us-east-2a",
APIURL: "https://api.starter-us-east-2a.openshift.com",
ConsoleURL: "https://console.starter-us-east-2a.openshift.com/console",
MetricsURL: "https://metrics.starter-us-east-2a.openshift.com",
LoggingURL: "https://console.starter-us-east-2a.openshift.com/console",
AppDNS: "b542.starter-us-east-2a.openshiftapps.com",
ServiceAccountToken: "ak61T6RSAacWFruh1vZP8cyUOBtQ3Chv1rdOBddSuc9nZ2wEcs81DHXRO55NpIpVQ8uiH",
ServiceAccountUsername: "dsaas",
TokenProviderID: "886c7ea3-ef97-443d-b345-de94b94bb65d",
AuthClientID: "autheast2a",
AuthClientSecret: "autheast2asecret",
AuthClientDefaultScope: "user:full",
Name: "us-east-2a",
APIURL: "https://api.starter-us-east-2a.openshift.com",
ConsoleURL: "https://console.starter-us-east-2a.openshift.com/console",
MetricsURL: "https://metrics.starter-us-east-2a.openshift.com",
LoggingURL: "https://console.starter-us-east-2a.openshift.com/console",
AppDNS: "b542.starter-us-east-2a.openshiftapps.com",
ServiceAccountToken: "ak61T6RSAacWFruh1vZP8cyUOBtQ3Chv1rdOBddSuc9nZ2wEcs81DHXRO55NpIpVQ8uiH",
ServiceAccountUsername: "dsaas",
ServiceAccountTokenEncrypted: configuration.IsSaTokenEncrypted(true),
TokenProviderID: "886c7ea3-ef97-443d-b345-de94b94bb65d",
AuthClientID: "autheast2a",
AuthClientSecret: "autheast2asecret",
AuthClientDefaultScope: "user:full",
Type: "OSO",
CapacityExhausted: false,
})
checkCluster(t, clusters, configuration.Cluster{
Name: "us-east-1a",
APIURL: "https://api.starter-us-east-1a.openshift.com",
ConsoleURL: "https://console.starter-us-east-1a.openshift.com/console",
MetricsURL: "https://metrics.starter-us-east-1a.openshift.com",
LoggingURL: "https://console.starter-us-east-1a.openshift.com/console",
AppDNS: "b542.starter-us-east-1a.openshiftapps.com",
ServiceAccountToken: "sdfjdlfjdfkjdlfjd12324434543085djdfjd084508gfdkjdofkjg43854085dlkjdlk",
ServiceAccountUsername: "dsaas",
TokenProviderID: "886c7ea3-ef97-443d-b345-de94b94bb65d",
AuthClientID: "autheast1a",
AuthClientSecret: "autheast1asecret",
AuthClientDefaultScope: "user:full",
Name: "us-east-1a",
APIURL: "https://api.starter-us-east-1a.openshift.com",
ConsoleURL: "https://console.starter-us-east-1a.openshift.com/console",
MetricsURL: "https://metrics.starter-us-east-1a.openshift.com",
LoggingURL: "https://console.starter-us-east-1a.openshift.com/console",
AppDNS: "b542.starter-us-east-1a.openshiftapps.com",
ServiceAccountToken: "sdfjdlfjdfkjdlfjd12324434543085djdfjd084508gfdkjdofkjg43854085dlkjdlk",
ServiceAccountUsername: "dsaas",
ServiceAccountTokenEncrypted: configuration.IsSaTokenEncrypted(false),
TokenProviderID: "886c7ea3-ef97-443d-b345-de94b94bb65d",
AuthClientID: "autheast1a",
AuthClientSecret: "autheast1asecret",
AuthClientDefaultScope: "user:full",
Type: "OSO",
CapacityExhausted: true,
})
checkCluster(t, clusters, configuration.Cluster{
Name: "us-east-3a",
APIURL: "https://api.starter-us-east-3a.openshift.com",
ConsoleURL: "https://console.starter-us-east-3a.openshift.com/console",
MetricsURL: "https://metrics.starter-us-east-3a.openshift.com",
LoggingURL: "https://console.starter-us-east-3a.openshift.com/console",
AppDNS: "b542.starter-us-east-3a.openshiftapps.com",
ServiceAccountToken: "fkdjhfdsjfgfdjlsflhjgsafgskfdsagrwgwerwshbdjasbdjbsahdbsagbdyhsbdesbh",
ServiceAccountUsername: "dsaas",
TokenProviderID: "1c09073a-13ad-4add-b0ff-197eaf18fc37",
AuthClientID: "autheast3a",
AuthClientSecret: "autheast3asecret",
AuthClientDefaultScope: "user:full",
Name: "us-east-3a",
APIURL: "https://api.starter-us-east-3a.openshift.com",
ConsoleURL: "https://console.starter-us-east-3a.openshift.com/console",
MetricsURL: "https://metrics.starter-us-east-3a.openshift.com",
LoggingURL: "https://console.starter-us-east-3a.openshift.com/console",
AppDNS: "b542.starter-us-east-3a.openshiftapps.com",
ServiceAccountToken: "fkdjhfdsjfgfdjlsflhjgsafgskfdsagrwgwerwshbdjasbdjbsahdbsagbdyhsbdesbh",
ServiceAccountUsername: "dsaas",
ServiceAccountTokenEncrypted: configuration.IsSaTokenEncrypted(true),
TokenProviderID: "1c09073a-13ad-4add-b0ff-197eaf18fc37",
AuthClientID: "autheast3a",
AuthClientSecret: "autheast3asecret",
AuthClientDefaultScope: "user:full",
Type: "OSD",
CapacityExhausted: false,
})
Expand Down
1 change: 1 addition & 0 deletions design/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ var fullClusterData = a.Type("FullClusterData", func() {

a.Attribute("service-account-token", d.String, "Decrypted cluster wide token")
a.Attribute("service-account-username", d.String, "Username of the cluster wide user")
a.Attribute("sa-token-encrypted", d.Boolean, "encrypted Service Account Token set to 'true'")
a.Attribute("token-provider-id", d.String, "Token provider ID")
a.Attribute("auth-client-id", d.String, "OAuth client ID")
a.Attribute("auth-client-secret", d.String, "OAuth client secret")
Expand Down
1 change: 1 addition & 0 deletions migration/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ func Steps() Scripts {
{"003-unique-index-on-cluster-api-url.sql"},
{"004-add-capacity-exhausted-to-cluster.sql"},
{"005-alter-cluster-api-url-index-to-unique.sql"},
{"006-add-sa-token-encrypted-to-cluster.sql"},
}
}

Expand Down
29 changes: 29 additions & 0 deletions migration/migration_blackbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func (s *MigrationTestSuite) TestMigrate() {
s.T().Run("testMigration003UniqueIndexOnClusterApiUrl", testMigration003UniqueIndexOnClusterApiUrl)
s.T().Run("testMigration004AddCapacityExhaustedToCluster", testMigration004AddCapacityExhaustedToCluster)
s.T().Run("testMigration005AlterClusterAPIURLIndexToUnique", testMigration005AlterClusterAPIURLIndexToUnique)
s.T().Run("testMigration006AddSaTokenEncryptedToCluster", testMigration006AddSaTokenEncryptedToCluster)
}

func testMigration001Cluster(t *testing.T) {
Expand Down Expand Up @@ -210,3 +211,31 @@ func testMigration005AlterClusterAPIURLIndexToUnique(t *testing.T) {

require.Error(t, err)
}

func testMigration006AddSaTokenEncryptedToCluster(t *testing.T) {
err := migrationsupport.Migrate(sqlDB, databaseName, migration.Steps()[:7])
require.NoError(t, err)

assert.True(t, dialect.HasColumn("cluster", "sa_token_encrypted"))

_, err = sqlDB.Exec(`INSERT INTO cluster (cluster_id, created_at, updated_at, name, url, console_url,
metrics_url, logging_url, app_dns, sa_token, sa_username, sa_token_encrypted, token_provider_id,
auth_client_id, auth_client_secret, auth_default_scope, type)
VALUES ('3eb0fb9a-b7d3-479f-9ec0-e5f93c7b1e53', now(), now(), 'exhausted', 'https://token-encrypted.api.cluster.com', 'https://console.cluster.com',
'https://metrics.cluster.com', 'https://login.cluster.com', 'https://app.cluster.com', 'sometoken', 'dssas-sre', false, 'pr-id',
'client-id', 'cleint-scr', 'somescope', 'OSD')`)
require.NoError(t, err)

// check if ALL the existing rows & new rows have the default value
rows, err := sqlDB.Query("SELECT sa_token_encrypted FROM cluster")
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

require.NoError(t, err)

t.Fatal(err)
}
defer rows.Close()
for rows.Next() {
var sa_token_encrypted bool
err = rows.Scan(&sa_token_encrypted)
require.NoError(t, err)
assert.False(t, sa_token_encrypted)
}
}
2 changes: 2 additions & 0 deletions migration/sql-files/006-add-sa-token-encrypted-to-cluster.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- Store sa_token_encrypted for cluster
ALTER TABLE cluster ADD COLUMN sa_token_encrypted boolean NOT NULL DEFAULT FALSE;
Copy link
Contributor

Choose a reason for hiding this comment

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

please don't use NOT NULL in this sql statement: if we need to rollback the deployment, we'll be in troubles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see reason to rollback the deployment. Do you see?

Copy link
Contributor

Choose a reason for hiding this comment

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

it's just a good practice IMO

3 changes: 3 additions & 0 deletions test/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func NewCluster() *repository.Cluster {
Name: uuid.NewV4().String(),
SaToken: uuid.NewV4().String(),
SaUsername: uuid.NewV4().String(),
SaTokenEncrypted: true,
TokenProviderID: uuid.NewV4().String(),
Type: uuid.NewV4().String(),
URL: uuid.NewV4().String(),
Expand All @@ -62,6 +63,7 @@ func AssertEqualClusterDetails(t *testing.T, expected, actual *repository.Cluste
assert.Equal(t, expected.TokenProviderID, actual.TokenProviderID)
assert.Equal(t, expected.SaUsername, actual.SaUsername)
assert.Equal(t, expected.SaToken, actual.SaToken)
assert.Equal(t, expected.SaTokenEncrypted, actual.SaTokenEncrypted)
assert.Equal(t, expected.Name, actual.Name)
assert.Equal(t, expected.MetricsURL, actual.MetricsURL)
assert.Equal(t, expected.LoggingURL, actual.LoggingURL)
Expand Down Expand Up @@ -148,6 +150,7 @@ func ClusterFromConfigurationCluster(configCluster configuration.Cluster) *repos

SaToken: configCluster.ServiceAccountToken,
SaUsername: configCluster.ServiceAccountUsername,
SaTokenEncrypted: *configCluster.ServiceAccountTokenEncrypted,
TokenProviderID: configCluster.TokenProviderID,
AuthClientID: configCluster.AuthClientID,
AuthClientSecret: configCluster.AuthClientSecret,
Expand Down