Skip to content

Commit

Permalink
Remove default azure secret name (kubeflow#315)
Browse files Browse the repository at this point in the history 
* add readme for azure

* fallback if the well known az secret is not found

* remove azure config name

* remove azure config
  • Loading branch information
@rakelkar @k8s-ci-robot
rakelkar authored and k8s-ci-robot committed Aug 27, 2019
1 parent 6503cb9 commit f31141e
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 24 deletions.
3 changes: 0 additions & 3 deletions config/default/configmap/kfservice.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,5 @@ data:
"s3": {
"s3AccessKeyIDName": "awsAccessKeyID",
"s3SecretAccessKeyName": "awsSecretAccessKey"
},
"azure": {
"azureSecretName": "azcreds"
}
}
2 changes: 1 addition & 1 deletion docs/samples/azure/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ KFServing supports authenticating using an Azure Service Principle.
* Details on assigning storage roles [here](https://docs.microsoft.com/en-us/azure/storage/common/storage-auth-aad).

### Create a K8s Secret
Store your Azure SP secrets as a k8s secret named `azcreds`.
Store your Azure SP secrets as a k8s secret.

```yaml
apiVersion: v1
Expand Down
7 changes: 1 addition & 6 deletions pkg/controller/kfservice/resources/credentials/azure/azure_secret.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,13 @@ import (
)

const (
AzureSecretName = "azcreds"
AzureSubscriptionId = "AZ_SUBSCRIPTION_ID"
AzureTenantId = "AZ_TENANT_ID"
AzureClientId = "AZ_CLIENT_ID"
AzureClientSecret = "AZ_CLIENT_SECRET"
)

type AzureConfig struct {
AzureSecretName string `json:"azureSecretName,omitempty"`
}

func BuildSecretEnvs(secret *v1.Secret, azureConfig *AzureConfig) []v1.EnvVar {
func BuildSecretEnvs(secret *v1.Secret) []v1.EnvVar {
envs := []v1.EnvVar{
{
Name: AzureSubscriptionId,
Expand Down
3 changes: 1 addition & 2 deletions pkg/controller/kfservice/resources/credentials/azure/azure_secret_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

func TestAzureSecret(t *testing.T) {
scenarios := map[string]struct {
config AzureConfig
secret *v1.Secret
expected []v1.EnvVar
}{
Expand Down Expand Up @@ -86,7 +85,7 @@ func TestAzureSecret(t *testing.T) {
}

for name, scenario := range scenarios {
envs := BuildSecretEnvs(scenario.secret, &scenario.config)
envs := BuildSecretEnvs(scenario.secret)

if diff := cmp.Diff(scenario.expected, envs); diff != "" {
t.Errorf("Test %q unexpected result (-want +got): %v", name, diff)
Expand Down
15 changes: 5 additions & 10 deletions pkg/controller/kfservice/resources/credentials/service_account_credentials.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@ const (
)

type CredentialConfig struct {
S3 s3.S3Config `json:"s3,omitempty"`
GCS gcs.GCSConfig `json:"gcs,omitempty"`
AZURE azure.AzureConfig `json:"azure,omitempty"`
S3 s3.S3Config `json:"s3,omitempty"`
GCS gcs.GCSConfig `json:"gcs,omitempty"`
}

type CredentialBuilder struct {
Expand Down Expand Up @@ -67,17 +66,12 @@ func (c *CredentialBuilder) CreateSecretVolumeAndEnv(namespace string, serviceAc
serviceAccountName = "default"
}
s3SecretAccessKeyName := s3.AWSSecretAccessKeyName
azureSecretName := azure.AzureSecretName
gcsCredentialFileName := gcs.GCSCredentialFileName

if c.config.S3.S3SecretAccessKeyName != "" {
s3SecretAccessKeyName = c.config.S3.S3SecretAccessKeyName
}

if c.config.AZURE.AzureSecretName != "" {
azureSecretName = c.config.AZURE.AzureSecretName
}

if c.config.GCS.GCSCredentialFileName != "" {
gcsCredentialFileName = c.config.GCS.GCSCredentialFileName
}
Expand All @@ -89,6 +83,7 @@ func (c *CredentialBuilder) CreateSecretVolumeAndEnv(namespace string, serviceAc
log.Error(err, "Failed to find service account", "ServiceAccountName", serviceAccountName)
return nil
}

for _, secretRef := range serviceAccount.Secrets {
log.Info("found secret", "SecretName", secretRef.Name)
secret := &v1.Secret{}
Expand All @@ -113,9 +108,9 @@ func (c *CredentialBuilder) CreateSecretVolumeAndEnv(namespace string, serviceAc
Name: gcs.GCSCredentialEnvKey,
Value: gcs.GCSCredentialVolumeMountPath + gcsCredentialFileName,
})
} else if secret.Name == azureSecretName {
} else if _, ok := secret.Data[azure.AzureClientSecret]; ok {
log.Info("Setting secret envs for azure", "AzureSecret", secret.Name)
envs := azure.BuildSecretEnvs(secret, &c.config.AZURE)
envs := azure.BuildSecretEnvs(secret)
container.Env = append(container.Env, envs...)
} else {
log.V(5).Info("Skipping non gcs/s3/azure secret", "Secret", secret.Name)
Expand Down
149 changes: 147 additions & 2 deletions pkg/controller/kfservice/resources/credentials/service_account_credentials_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"context"
"testing"

"github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials/azure"

"github.com/google/go-cmp/cmp"
"github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials/gcs"
"github.com/kubeflow/kfserving/pkg/controller/kfservice/resources/credentials/s3"
Expand All @@ -37,8 +39,7 @@ var configMap = &v1.ConfigMap{
"s3" : {
"s3AccessKeyIDName": "awsAccessKeyID",
"s3SecretAccessKeyName": "awsSecretAccessKey"
},
"azure" : {"azureSecretName": "azcreds"}
}
}`,
},
}
Expand Down Expand Up @@ -287,3 +288,147 @@ func TestGCSCredentialBuilder(t *testing.T) {

}
}

func TestAzureCredentialBuilder(t *testing.T) {
g := gomega.NewGomegaWithT(t)
customOnlyServiceAccount := &v1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: "custom-sa",
Namespace: "default",
},
Secrets: []v1.ObjectReference{
{
Name: "az-custom-secret",
Namespace: "default",
},
},
}
customAzureSecret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "az-custom-secret",
Namespace: "default",
},
Data: map[string][]byte{
"AZ_SUBSCRIPTION_ID": {},
"AZ_TENANT_ID": {},
"AZ_CLIENT_ID": {},
"AZ_CLIENT_SECRET": {},
},
}

scenarios := map[string]struct {
serviceAccount *v1.ServiceAccount
inputConfiguration *v1alpha1.Configuration
expectedConfiguration *v1alpha1.Configuration
shouldFail bool
}{
"Custom Azure Secret": {
serviceAccount: customOnlyServiceAccount,
inputConfiguration: &v1alpha1.Configuration{
Spec: v1alpha1.ConfigurationSpec{
Template: &v1alpha1.RevisionTemplateSpec{
Spec: v1alpha1.RevisionSpec{
RevisionSpec: v1beta1.RevisionSpec{
PodSpec: v1.PodSpec{
Containers: []v1.Container{
{},
},
},
},
},
},
},
},
expectedConfiguration: &v1alpha1.Configuration{
Spec: v1alpha1.ConfigurationSpec{
Template: &v1alpha1.RevisionTemplateSpec{
Spec: v1alpha1.RevisionSpec{
RevisionSpec: v1beta1.RevisionSpec{
PodSpec: v1.PodSpec{
Containers: []v1.Container{
{
Env: []v1.EnvVar{
{
Name: azure.AzureSubscriptionId,
ValueFrom: &v1.EnvVarSource{
SecretKeyRef: &v1.SecretKeySelector{
LocalObjectReference: v1.LocalObjectReference{
Name: "az-custom-secret",
},
Key: azure.AzureSubscriptionId,
},
},
},
{
Name: azure.AzureTenantId,
ValueFrom: &v1.EnvVarSource{
SecretKeyRef: &v1.SecretKeySelector{
LocalObjectReference: v1.LocalObjectReference{
Name: "az-custom-secret",
},
Key: azure.AzureTenantId,
},
},
},
{
Name: azure.AzureClientId,
ValueFrom: &v1.EnvVarSource{
SecretKeyRef: &v1.SecretKeySelector{
LocalObjectReference: v1.LocalObjectReference{
Name: "az-custom-secret",
},
Key: azure.AzureClientId,
},
},
},
{
Name: azure.AzureClientSecret,
ValueFrom: &v1.EnvVarSource{
SecretKeyRef: &v1.SecretKeySelector{
LocalObjectReference: v1.LocalObjectReference{
Name: "az-custom-secret",
},
Key: azure.AzureClientSecret,
},
},
},
},
},
},
},
},
},
},
},
},
shouldFail: false,
},
}

g.Expect(c.Create(context.TODO(), customAzureSecret)).NotTo(gomega.HaveOccurred())
g.Expect(c.Create(context.TODO(), customOnlyServiceAccount)).NotTo(gomega.HaveOccurred())

builder := NewCredentialBulder(c, configMap)
for name, scenario := range scenarios {

err := builder.CreateSecretVolumeAndEnv(scenario.serviceAccount.Namespace, scenario.serviceAccount.Name,
&scenario.inputConfiguration.Spec.Template.Spec.Containers[0],
&scenario.inputConfiguration.Spec.Template.Spec.Volumes,
)
if scenario.shouldFail && err == nil {
t.Errorf("Test %q failed: returned success but expected error", name)
}
// Validate
if !scenario.shouldFail {
if err != nil {
t.Errorf("Test %q failed: returned error: %v", name, err)
}
if diff := cmp.Diff(scenario.expectedConfiguration, scenario.inputConfiguration); diff != "" {
t.Errorf("Test %q unexpected configuration spec (-want +got): %v", name, diff)
}
}
}

g.Expect(c.Delete(context.TODO(), customAzureSecret)).NotTo(gomega.HaveOccurred())
g.Expect(c.Delete(context.TODO(), customOnlyServiceAccount)).NotTo(gomega.HaveOccurred())
}

0 comments on commit f31141e

Please sign in to comment.