diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 042565339..f44649e41 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -398,17 +398,11 @@ Example POSTs (non-trivial as it involves the URL map): "name": "mysecretname", "username": "myusername", "password": "mypassword", - "type": "userpass", "description": "my secret for github", "url": {"tekton.dev/git-0": "https://github.com"} "serviceAccount": "sa1" } -{ - "name": "mytoken", - "type", "accesstoken", - "accesstoken": "34ecf56accce4" -} ``` __PipelineRuns__ diff --git a/pkg/endpoints/credentials.go b/pkg/endpoints/credentials.go index 17e0b3ffb..ba1de9ee8 100644 --- a/pkg/endpoints/credentials.go +++ b/pkg/endpoints/credentials.go @@ -30,14 +30,8 @@ import ( type credential struct { Name string `json:"name,omitempty"` - Type string `json:"type"` // must have the value 'accesstoken', 'userpass' or 'dockerregistry' Username string `json:"username,omitempty"` Password string `json:"password,omitempty"` - AccessToken string `json:"accesstoken,omitempty"` - DockerServer string `json:"dockerserver,omitempty"` - DockerUsername string `json:"dockerusername,omitempty"` - DockerEmail string `json:"dockeruseremail,omitempty"` - DockerPassword string `json:"dockerpassword,omitempty"` Description string `json:"description,omitempty"` ResourceVersion string `json:"resourceVersion,omitempty"` URL map[string]string `json:"url,omitempty"` @@ -45,12 +39,6 @@ type credential struct { } const ( - typeAccessToken string = "accesstoken" - typeUserPass string = "userpass" - typeDockerRegistry string = "dockerregistry" - dashboardKey string = "tekton-dashboard" - dashboardValue string = "true" - dashboardLabelSelector string = dashboardKey + "=" + dashboardValue // must have format "=" serviceAccountLabelKey string = "service-account" ) @@ -68,7 +56,7 @@ func (r Resource) getAllCredentials(request *restful.Request, response *restful. } // Get secrets from the resource K8sClient - secrets, err := r.K8sClient.CoreV1().Secrets(requestNamespace).List(metav1.ListOptions{LabelSelector: dashboardLabelSelector}) + secrets, err := r.K8sClient.CoreV1().Secrets(requestNamespace).List(metav1.ListOptions{FieldSelector: "type=kubernetes.io/basic-auth"}) if err != nil { errorMessage := fmt.Sprintf("Error getting secrets from K8sClient: %s.", err.Error()) response.WriteErrorString(http.StatusInternalServerError, errorMessage) @@ -296,28 +284,17 @@ func (r Resource) verifySecretExists(secretName string, namespace string, respon /* Verify required parameters are in credential struct * Required parameters: - * - Type (must have the value 'accesstoken' 'userpass' or 'dockerregistry) * - Name * If type == userpass * - Username * - Password - * else if type == accesstoken - * - AccessToken - * else if type == dockerregistry - * - DockerServer - * - DockerUsername - * - DockerPassword */ func (r Resource) verifyCredentialParameters(cred credential, response *restful.Response) bool { errorMessage := "" - if cred.Name == "" || cred.Type == "" || (cred.Type != typeAccessToken && cred.Type != typeUserPass && cred.Type != typeDockerRegistry) { - errorMessage = fmt.Sprintf("Error: Name and Type ('%s', '%s' or '%s' must be specified", typeAccessToken, typeUserPass, typeDockerRegistry) - } else if cred.Type == typeUserPass && (cred.Username == "" || cred.Password == "") { - errorMessage = fmt.Sprintf("Error: Username and Password must all be supplied for Type %s", cred.Type) - } else if cred.Type == typeAccessToken && cred.AccessToken == "" { - errorMessage = fmt.Sprintf("Error: AccessToken must be supplied for Type %s", cred.Type) - } else if cred.Type == typeDockerRegistry && (cred.DockerServer == "" || cred.DockerUsername == "" || cred.DockerPassword == "") { - errorMessage = fmt.Sprintf("Error: DockerServer, Username and Password must be supplied for Type %s", cred.Type) + if cred.Name == "" { + errorMessage = fmt.Sprintf("Error: Name must be specified") + } else if cred.Username == "" || cred.Password == "" { + errorMessage = fmt.Sprintf("Error: Username and Password must all be supplied") } for key := range cred.URL { if strings.Index(key, "tekton.dev/docker-") != 0 && strings.Index(key, "tekton.dev/git-") != 0 { @@ -348,30 +325,14 @@ func getQueryEntity(entityPointer interface{}, request *restful.Request, respons func secretToCredential(secret *corev1.Secret, mask bool) credential { labels := secret.GetLabels() saName, _ := labels[serviceAccountLabelKey] - var cred credential - switch secretType := string(secret.Data["type"]); secretType { - case typeUserPass: - cred = credential{ - Name: secret.GetName(), - Username: string(secret.Data["username"]), - Password: string(secret.Data["password"]), - Description: string(secret.Data["description"]), - Type: secretType, - URL: secret.ObjectMeta.Annotations, - ResourceVersion: secret.GetResourceVersion(), - ServiceAccount: saName, - } - case typeAccessToken: - cred = credential{ - Name: secret.GetName(), - AccessToken: string(secret.Data["accesstoken"]), - Description: string(secret.Data["description"]), - Type: secretType, - URL: secret.ObjectMeta.Annotations, - ResourceVersion: secret.GetResourceVersion(), - ServiceAccount: saName, - } - // TODO: support for typeDockerRegistry + cred := credential{ + Name: secret.GetName(), + Username: string(secret.Data["username"]), + Password: string(secret.Data["password"]), + Description: string(secret.Data["description"]), + URL: secret.ObjectMeta.Annotations, + ResourceVersion: secret.GetResourceVersion(), + ServiceAccount: saName, } if mask { cred.Password = "********" @@ -386,19 +347,10 @@ func credentialToSecret(cred credential, namespace string, response *restful.Res secret.SetNamespace(namespace) secret.SetName(cred.Name) secret.Data = make(map[string][]byte) - switch cred.Type { - case typeUserPass: - secret.Type = corev1.SecretTypeBasicAuth - secret.Data["username"] = []byte(cred.Username) - secret.Data["password"] = []byte(cred.Password) - case typeAccessToken: - secret.Type = corev1.SecretTypeOpaque - secret.Data["accesstoken"] = []byte(cred.AccessToken) - default: - // TODO: add support for corev1.SecretTypeDockerConfigJson - } + secret.Type = corev1.SecretTypeBasicAuth + secret.Data["username"] = []byte(cred.Username) + secret.Data["password"] = []byte(cred.Password) secret.Data["description"] = []byte(cred.Description) - secret.Data["type"] = []byte(cred.Type) secret.ObjectMeta.Annotations = cred.URL saName := "" if cred.ServiceAccount != "" { @@ -411,9 +363,7 @@ func credentialToSecret(cred credential, namespace string, response *restful.Res saName = "default" } } - labels := map[string]string{ - dashboardKey: dashboardValue, serviceAccountLabelKey: saName, } secret.SetLabels(labels) diff --git a/pkg/endpoints/credentials_test.go b/pkg/endpoints/credentials_test.go index fdce8df1f..8c57dd49c 100644 --- a/pkg/endpoints/credentials_test.go +++ b/pkg/endpoints/credentials_test.go @@ -65,93 +65,68 @@ func TestCredentials(t *testing.T) { r.K8sClient.CoreV1().Namespaces().Create(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}) // Initialize test data - expectCreds := []credential{} - accessTokenCred := credential{ - Name: "credentialaccesstoken", - Type: "accesstoken", - AccessToken: "personal-access-token", - Description: "access token credential", - URL: map[string]string{ - "tekton.dev/git-0": "https://github.com", - }, - ServiceAccount: "sa1", - } userPassCred := credential{ Name: "credentialuserpass", Username: "usernameuserpass", Password: "passworduserpass", Description: "user password credential", - Type: "userpass", URL: map[string]string{ "tekton.dev/docker-0": "https://gcr.io", }, ServiceAccount: "default", } + anotherBasicCred := credential{ + Name: "gitcreds", + Username: "gitusername", + Password: "gitpassword", + Description: "second user password credential", + URL: map[string]string{ + "tekton.dev/git-0": "https://my.git.org", + }, + ServiceAccount: "default", + } // READ ALL credentials when there are none + t.Log("READ all credentials when there are none") + expectCreds := []credential{} readAllCredentialsTest(namespace, expectCreds, "", r, t) - // CREATE credential accesstoken - t.Log("CREATE credential accesstoken") - createCredentialTest(namespace, accessTokenCred, "", r, t) - if !r.checkSecretInSa(namespace, "sa1", "credentialaccesstoken", t) { - t.Fatal("FAIL: ERROR - service account doesn't have the secret: credentialaccesstoken") - } - - // CREATE credential userpass + // CREATE credential userpass, anotherBasicCred t.Log("CREATE credential userpass") createCredentialTest(namespace, userPassCred, "", r, t) if !r.checkSecretInSa(namespace, "default", "credentialuserpass", t) { t.Fatal("FAIL: ERROR - service account doesn't have the secret: credentialuserpass") } - // READ ALL credentials when there is the accesstoken credential and userpass credential - t.Log("READ all credentials when there is 'credentialaccesstoken' and 'credentialuserpass'") - expectCreds = []credential{ - accessTokenCred, - userPassCred, + t.Log("CREATE credential gitcreds") + createCredentialTest(namespace, anotherBasicCred, "", r, t) + if !r.checkSecretInSa(namespace, "default", "gitcreds", t) { + t.Fatal("FAIL: ERROR - service account doesn't have the secret: gitcreds") } - readAllCredentialsTest(namespace, expectCreds, "", r, t) - - // READ credential accesstoken - t.Log("READ credential 'credentialaccesstoken' when it exists") - // Create dummy rest api request and response - readCredentialTest(namespace, accessTokenCred, "", r, t) // READ credential userpass t.Log("GET credential 'credentialuserpass' when it exists") readCredentialTest(namespace, userPassCred, "", r, t) - // UPDATE credential - t.Log("UPDATE credential 'credentialaccesstoken' when it exists") - accessTokenCred.AccessToken = "personal-access-token-updated" - accessTokenCred.Description = "access token credential updated" - accessTokenCred.Type = "accesstoken" - updateCredentialTest(namespace, accessTokenCred, "", r, t) - // UPDATE credential userpass t.Log("UPDATE credential 'credentialuserpass' when it exists") userPassCred.Username = "usernameuserpassupdated" userPassCred.Password = "passworduserpassupdated" userPassCred.Description = "user password credential updated" - userPassCred.Type = "userpass" updateCredentialTest(namespace, userPassCred, "", r, t) - - // READ ALL - t.Log("READ all credentials when there is 'credentialaccesstoken' and 'credentialuserpass' (they were both just updated)") expectCreds = []credential{ - accessTokenCred, userPassCred, + anotherBasicCred, } readAllCredentialsTest(namespace, expectCreds, "", r, t) // DELETE credential accesstoken t.Log("DELETE credential 'credentialaccesstoken' when it exists") - deleteCredentialTest(namespace, accessTokenCred.Name, "", r, t) + deleteCredentialTest(namespace, anotherBasicCred.Name, "", r, t) // READ ALL credentials when there is only the userpass credential - t.Log("READ all credentials when there is only 'credentialuserpass' ('credentialaccesstoken' was just deleted)") + t.Log("READ all credentials when there is only 'credentialuserpass' ('gitcreds' was just deleted)") expectCreds = []credential{ userPassCred, } @@ -167,23 +142,18 @@ func TestCredentials(t *testing.T) { readAllCredentialsTest(namespace, expectCreds, "", r, t) } -// Check error reporting for credentials created with no name or type, or bad types -func TestCreateCredentialsWithNoNameOrType(t *testing.T) { +// Check error reporting for credentials created with no name +func TestCreateCredentialWithNoName(t *testing.T) { r := dummyResource() namespace := "tekton-pipelines" r.K8sClient.CoreV1().Namespaces().Create(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}) invalidCreds := []credential{ credential{ - Type: "accesstoken", - }, - credential{ - Name: "goodNameNoType", - }, - credential{ - Type: "badtype", + Username: "myname", + Password: "mypassword", }, } - expectedError := fmt.Sprintf("Error: Name and Type ('%s', '%s' or '%s' must be specified", typeAccessToken, typeUserPass, typeDockerRegistry) + expectedError := fmt.Sprintf("Error: Name must be specified") for _, invalidCred := range invalidCreds { createCredentialTest(namespace, invalidCred, expectedError, r, t) updateCredentialTest(namespace, invalidCred, expectedError, r, t) @@ -199,16 +169,14 @@ func TestCreateBadUserPass(t *testing.T) { badUserPass := []credential{ credential{ Name: "badCredNoUsername", - Type: "userpass", Password: "badpw1", }, credential{ Name: "badCredNoPassword", - Type: "userpass", Username: "someone", }, } - expectedError := fmt.Sprintf("Error: Username and Password must all be supplied for Type %s", typeUserPass) + expectedError := fmt.Sprintf("Error: Username and Password must all be supplied") for _, invalidCred := range badUserPass { createCredentialTest(namespace, invalidCred, expectedError, r, t) updateCredentialTest(namespace, invalidCred, expectedError, r, t) @@ -217,55 +185,6 @@ func TestCreateBadUserPass(t *testing.T) { readAllCredentialsTest(namespace, []credential{}, "", r, t) } -func TestCreateBadAccessToken(t *testing.T) { - r := dummyResource() - namespace := "access-tokens" - r.K8sClient.CoreV1().Namespaces().Create(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}) - badAccessToken := credential{ - Name: "badToken", - Type: "accesstoken", - Password: "shouldHaveBeenAnAccessToken", - } - expectedError := fmt.Sprintf("Error: AccessToken must be supplied for Type %s", typeAccessToken) - createCredentialTest(namespace, badAccessToken, expectedError, r, t) - updateCredentialTest(namespace, badAccessToken, expectedError, r, t) - // Verify no credentials have been created - readAllCredentialsTest(namespace, []credential{}, "", r, t) -} - -func TestCreateBadDockerSecrets(t *testing.T) { - r := dummyResource() - namespace := "docker-secrets" - r.K8sClient.CoreV1().Namespaces().Create(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}}) - badDockerCreds := []credential{ - credential{ - Name: "noDockerServer", - Type: "dockerregistry", - DockerUsername: "name", - DockerPassword: "pass", - }, - credential{ - Name: "noDockerUsername", - Type: "dockerregistry", - DockerServer: "server", - DockerPassword: "pass", - }, - credential{ - Name: "noDockerPassword", - Type: "dockerregistry", - DockerUsername: "name2", - DockerPassword: "pass", - }, - } - expectedError := fmt.Sprintf("Error: DockerServer, Username and Password must be supplied for Type %s", typeDockerRegistry) - for _, invalidCred := range badDockerCreds { - createCredentialTest(namespace, invalidCred, expectedError, r, t) - updateCredentialTest(namespace, invalidCred, expectedError, r, t) - } - // Verify no credentials have been created - readAllCredentialsTest(namespace, []credential{}, "", r, t) -} - func TestCreateCredentialsWithBadUrls(t *testing.T) { r := dummyResource() namespace := "tekton-pipelines" @@ -274,7 +193,6 @@ func TestCreateCredentialsWithBadUrls(t *testing.T) { credsWithBadUrls := []credential{ credential{ Name: "userpass", - Type: "userpass", Username: "user", Password: "pass", URL: map[string]string{ @@ -282,13 +200,11 @@ func TestCreateCredentialsWithBadUrls(t *testing.T) { }, }, credential{ - Name: "docker", - Type: "dockerregistry", - DockerServer: "my.docker.reg", - DockerUsername: "myName", - DockerPassword: "pass", + Name: "userpass2", + Username: "user2", + Password: "pass2", URL: map[string]string{ - "tekton/docker-0": "https://my.docker.registry.com", + "tekton.dev/dckr-0": "https://docker.io", }, }, } @@ -309,10 +225,9 @@ func TestCredentialsRUDThatDoNotExist(t *testing.T) { bogusNamespace := "bogusnamespace" expectedError := fmt.Sprintf("Namespace provided does not exist: '%s'.", bogusNamespace) cred := credential{ - Name: "credentialaccesstoken", - Type: "accesstoken", - AccessToken: "personal-access-token", - Description: "access token credential", + Name: "aCred", + Username: "alfred", + Password: "alfredsPassword", URL: map[string]string{ "tekton.dev/git-0": "https://github.com", }, @@ -537,9 +452,9 @@ func testCredential(resultCred credential, expectCred credential, t *testing.T) } } -// Get all K8s client secrets with selector LABEL_SELECTOR +// Get all K8s client secrets func (r Resource) getK8sCredentials(namespace string) (credentials []credential) { - secrets, err := r.K8sClient.CoreV1().Secrets(namespace).List(metav1.ListOptions{LabelSelector: dashboardLabelSelector}) + secrets, err := r.K8sClient.CoreV1().Secrets(namespace).List(metav1.ListOptions{FieldSelector: "type=kubernetes.io/basic-auth"}) if err != nil { return } diff --git a/pkg/endpoints/routes_test.go b/pkg/endpoints/routes_test.go index 61ba7746d..15c472214 100644 --- a/pkg/endpoints/routes_test.go +++ b/pkg/endpoints/routes_test.go @@ -253,9 +253,9 @@ func fakeCRD(t *testing.T, crdType string, identifier string) interface{} { case "credentials": return &credential{ Name: "fakeresource", - AccessToken: "thisismyaccesstoken", - Description: "access token de jour", - Type: "accesstoken", + Username: "thisismyusername", + Password: "password", + Description: "cred de jour", URL: map[string]string{ "tekton.dev/git-0": "https://github.com", },