-
Notifications
You must be signed in to change notification settings - Fork 529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JENKINS-41348# BlueOcean specific CredentialsProvider #768
Changes from 1 commit
954f3ce
58036dd
fda690a
cdd7878
26ff252
ac94b40
afb960f
bd6487b
5be8e09
52995e2
e9a6c4c
0579ef4
fd576c3
8530fba
8aff779
def90b4
de70584
eb0d942
da6251e
c02467f
13cba24
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,8 @@ | |
|
||
import com.cloudbees.hudson.plugins.folder.AbstractFolder; | ||
import com.cloudbees.plugins.credentials.Credentials; | ||
import com.cloudbees.plugins.credentials.domains.Domain; | ||
import com.cloudbees.plugins.credentials.domains.DomainRequirement; | ||
import hudson.model.Cause; | ||
import hudson.model.TopLevelItem; | ||
import hudson.model.User; | ||
|
@@ -79,12 +81,24 @@ public BluePipeline create(Reachable parent) throws IOException { | |
if(credentialId != null) { | ||
validateCredentialId(credentialId, (AbstractFolder) item); | ||
|
||
((OrganizationFolder) item) | ||
.addProperty( | ||
new BlueOceanCredentialsProvider.FolderPropertyImpl( | ||
authenticatedUser.getId(), credentialId, | ||
GithubCredentialsDomain(apiUrl) | ||
)); | ||
//Find domain attached to this credentialId, if present check if it's BlueOcean specific domain then | ||
//add the properties otherwise simply use it | ||
Domain domain = CredentialsUtils.findDomain(credentialId, authenticatedUser); | ||
if(domain == null){ //this should not happen since validateCredentialId found the credential | ||
throw new ServiceException.BadRequestExpception( | ||
new ErrorMessage(400, "Failed to create pipeline") | ||
.add(new ErrorMessage.Error("scm.credentialId", | ||
ErrorMessage.Error.ErrorCodes.INVALID.toString(), | ||
"No domain in user credentials found for credentialId: "+ scmConfig.getCredentialId()))); | ||
} | ||
if(domain.test(new DomainRequirement())) { | ||
((OrganizationFolder) item) | ||
.addProperty( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does addProperty handle duplicates correctly? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point, will add validation of this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No duplicate test needed, this code creates a new org folder and is add property right after creation. |
||
new BlueOceanCredentialsProvider.FolderPropertyImpl( | ||
authenticatedUser.getId(), credentialId, | ||
BlueOceanCredentialsProvider.createDomain(apiUrl) | ||
)); | ||
} | ||
} | ||
GitHubSCMNavigator gitHubSCMNavigator = new GitHubSCMNavigator(apiUrl, orgName, credentialId, credentialId); | ||
if (sb.length() > 0) { | ||
|
@@ -98,9 +112,6 @@ public BluePipeline create(Reachable parent) throws IOException { | |
return new GithubOrganizationFolder(organizationFolder, parent.getLink()); | ||
} | ||
} catch (Exception e){ | ||
if(e instanceof ServiceException){ | ||
throw e; | ||
} | ||
String msg = String.format("Error creating pipeline %s: %s",getName(),e.getMessage()); | ||
logger.error(msg, e); | ||
if(item != null) { | ||
|
@@ -112,6 +123,9 @@ public BluePipeline create(Reachable parent) throws IOException { | |
|
||
} | ||
} | ||
if(e instanceof ServiceException){ | ||
throw e; | ||
} | ||
throw new ServiceException.UnexpectedErrorException(msg, e); | ||
} | ||
return null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,11 @@ | |
import hudson.model.User; | ||
import io.jenkins.blueocean.rest.impl.pipeline.PipelineBaseTest; | ||
import io.jenkins.blueocean.rest.impl.pipeline.credential.BlueOceanCredentialsProvider; | ||
import io.jenkins.blueocean.rest.impl.pipeline.credential.BlueOceanDomainRequirement; | ||
import io.jenkins.blueocean.rest.impl.pipeline.credential.BlueOceanDomainSpecification; | ||
import io.jenkins.blueocean.rest.impl.pipeline.credential.CredentialsUtils; | ||
import jenkins.branch.OrganizationFolder; | ||
import jenkins.model.Jenkins; | ||
import org.jenkinsci.plugins.github_branch_source.Connector; | ||
import org.junit.Assert; | ||
import org.junit.Test; | ||
|
@@ -89,37 +93,52 @@ public void shouldFindUserStoreCredential() throws IOException { | |
break; | ||
} | ||
} | ||
|
||
assertNotNull(store); | ||
store.addDomain(new Domain("github-domain", | ||
"Github Domain to store personal access token", | ||
Collections.<DomainSpecification>emptyList() | ||
)); | ||
Collections.<DomainSpecification>singletonList(new BlueOceanDomainSpecification()))); | ||
|
||
|
||
Domain domain = store.getDomainByName("github-domain"); | ||
StandardUsernamePasswordCredentials credential = new UsernamePasswordCredentialsImpl(CredentialsScope.USER, | ||
"github", "Github Access Token", user.getId(), "12345"); | ||
store.addCredentials(domain, credential); | ||
|
||
//create another credentials with same id in system store with different description | ||
for(CredentialsStore s: CredentialsProvider.lookupStores(Jenkins.getInstance())){ | ||
s.addCredentials(Domain.global(), new UsernamePasswordCredentialsImpl(CredentialsScope.USER, | ||
"github", "System Github Access Token", user.getId(), "12345")); | ||
} | ||
|
||
//create org folder and attach user and credential id to it | ||
OrganizationFolder organizationFolder = j.createProject(OrganizationFolder.class, "demo"); | ||
AbstractFolderProperty prop = new BlueOceanCredentialsProvider.FolderPropertyImpl(user.getId(), credential.getId(), "github-domain" | ||
AbstractFolderProperty prop = new BlueOceanCredentialsProvider.FolderPropertyImpl(user.getId(), credential.getId(), | ||
BlueOceanCredentialsProvider.createDomain("https://api.github.com")); | ||
|
||
); | ||
organizationFolder.addProperty(prop); | ||
|
||
// lookup for created credential id in system store, it should resolve to previously created user store credential | ||
StandardCredentials c = Connector.lookupScanCredentials(organizationFolder, null, credential.getId()); | ||
StandardCredentials c = Connector.lookupScanCredentials(organizationFolder, "https://api.github.com", credential.getId()); | ||
assertEquals("Github Access Token", c.getDescription()); | ||
|
||
assertNotNull(c); | ||
assertTrue(c instanceof StandardUsernamePasswordCredentials); | ||
StandardUsernamePasswordCredentials usernamePasswordCredentials = (StandardUsernamePasswordCredentials) c; | ||
assertEquals(credential.getId(), usernamePasswordCredentials.getId()); | ||
assertEquals(credential.getPassword().getPlainText(),usernamePasswordCredentials.getPassword().getPlainText()); | ||
assertEquals(credential.getUsername(),usernamePasswordCredentials.getUsername()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing a test where there is a credential with the same ID in the system store, verifying that your proxy store is a higher ordinal and hence will get asked first |
||
|
||
//check the domain | ||
Domain d = CredentialsUtils.findDomain(credential.getId(), user); | ||
assertNotNull(d); | ||
assertTrue(d.test(new BlueOceanDomainRequirement())); | ||
|
||
//now remove this property | ||
organizationFolder.getProperties().remove(prop); | ||
|
||
//it must not be found | ||
//it must resolve to system credential | ||
c = Connector.lookupScanCredentials(organizationFolder, null, credential.getId()); | ||
assertNull(c); | ||
assertEquals("System Github Access Token", c.getDescription()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
BlueOceanCredentialsProvider.DisplayName=BlueOcean Folder Credentials | ||
BlueOceanCredentialsProvider.DisplayName=BlueOcean Folder Credentials | ||
BlueOceanCredentialsProvider.DomainDescription=Blue Ocean Folder Credentials domain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a bug, you should be testing against your
BlueOceanDomainRequirement
notDomainRequirement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it is, good catch.