[JENKINS-50204] Implemented the Credentials view page #7
Conversation
| @@ -67,6 +67,10 @@ | |||
| <id>teilo</id> | |||
| <name>James Nord</name> | |||
| </developer> | |||
| <developer> | |||
There was a problem hiding this comment.
can you request access so you can release as well. https://github.com/jenkins-infra/repository-permissions-updater/blob/master/permissions/plugin-kubernetes-credentials-provider.yml#L6
There was a problem hiding this comment.
Dockerfile
Outdated
| @@ -1,4 +1,4 @@ | |||
| FROM jenkins/jenkins:lts | |||
|
|
|||
| RUN /usr/local/bin/install-plugins.sh credentials:2.1.16 credentials-binding:1.15 | |||
| COPY target/kubernetes-credential-provider.hpi /usr/share/jenkins/ref/plugins/kubernetes-credential-provider.jpi | |||
| COPY target/kubernetes-credentials-provider.hpi /usr/share/jenkins/ref/plugins/kubernetes-credential-provider.jpi | |||
| import edu.umd.cs.findbugs.annotations.NonNull; | ||
| import edu.umd.cs.findbugs.annotations.Nullable; | ||
| import org.acegisecurity.Authentication; | ||
| import org.apache.commons.lang.NotImplementedException; |
There was a problem hiding this comment.
not the exception you are looking for?
|
|
||
| @Override | ||
| public boolean addCredentials(@NonNull Domain domain, @NonNull Credentials credentials) { | ||
| throw new NotImplementedException(); |
There was a problem hiding this comment.
should only throw an IOException (as per parent signature)
There was a problem hiding this comment.
NotImplementedException (and your suggestion of UnsupportedOperationException for that matter) are both RuntimeExceptions
|
|
||
| @Override | ||
| public boolean removeCredentials(@NonNull Domain domain, @NonNull Credentials credentials) { | ||
| throw new NotImplementedException(); |
There was a problem hiding this comment.
should only throw an IOException (as per parent signature)
This allows inspecting of individual secrets when clicking on them in the credentials view instead of receiving a big stapler stack trace
| @Override | ||
| public boolean updateCredentials(@NonNull Domain domain, @NonNull Credentials current, | ||
| @NonNull Credentials replacement) { | ||
| throw new NotImplementedException(); |
There was a problem hiding this comment.
should only throw an IOException (as per parent signature)
| */ | ||
| @Override | ||
| public String getIconClassName() { | ||
| return isVisible() |
There was a problem hiding this comment.
|
|
||
| @Override | ||
| public boolean hasPermission(@NonNull Authentication authentication, @NonNull Permission permission) { | ||
| return Jenkins.getInstance().getACL().hasPermission(authentication, permission); |
There was a problem hiding this comment.
As we don;t support Credential.ADD / UPDATE / REMOVE you should check these and return false (otherwise you would see the stack trace when you try to update / save)
| @NonNull | ||
| @Override | ||
| public List<Credentials> getCredentials(@NonNull Domain domain) { | ||
| // TODO: Filter by domain - how do I do this? |
There was a problem hiding this comment.
Domain.global is a static I think you can use
| } | ||
|
|
||
| /** | ||
| * {@inheritDoc} |
| * {@inheritDoc} | ||
| */ | ||
| @NonNull | ||
| public CredentialsStore getStore() { |
|
@agentgonzo can you add some screenshots please? |
|
|
|
|
| return provider.getCredentials(Credentials.class, Jenkins.getInstance(), ACL.SYSTEM); | ||
| // Only the global domain is supported | ||
| return Domain.global().equals(domain) | ||
| ? provider.getCredentials(Credentials.class, Jenkins.getInstance(), ACL.SYSTEM) |
There was a problem hiding this comment.
Ahh I think you need to pass jenkins.getAuthentication() rather than ACL.SYSTEM
There was a problem hiding this comment.
If I do that, then this check fails and we don't see the credentials
There was a problem hiding this comment.
Hmm.. I used this code as a reference for that.
JENKINS-50204
@jtnord