-
Notifications
You must be signed in to change notification settings - Fork 60
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
Toggle LDAP/ACL #40
base: master
Are you sure you want to change the base?
Toggle LDAP/ACL #40
Conversation
13151d6
to
292cdd0
Compare
} | ||
credentialsBinding { | ||
usernamePassword("LDAP_ADMIN_USER", "LDAP_ADMIN_PASSWORD", "adop-ldap-admin") | ||
if("${ADOP_LDAP_ENABLED}" == "true") |
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.
This variable ADOP_LDAP_ENABLED
needs to appear:
a) In the compose file for jenkins
b) Injected as a global env variable here: https://github.com/Accenture/adop-jenkins/blob/master/resources/init.groovy.d/adop_general.groovy
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.
- It'll also need the ACL env variable defined in the compose file and injected.
- Strings should also use content rather than reference equality.
41d55e2
to
1bc4911
Compare
I'm wondering if we should flip the logic in the interest of backwards compatibility? If someone is on an older version of the Jenkins image they won't have the environment variables which means the jobs will be generated with sections missing and it won't be obvious why. |
${WORKSPACE}/common/gerrit/create_user.sh -g http://gerrit:8080/gerrit -u "${username}" -p "${username}" | ||
done''') | ||
dsl { | ||
external("workspaces/jobs/**/*.groovy") |
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.
We need this line!
workspaces/jobs/jobs.groovy
Outdated
# Generate second permission repo with enabled code-review | ||
source ${WORKSPACE}/projects/gerrit/configure.sh -r permissions-with-review''') | ||
dsl { | ||
external("projects/jobs/**/*.groovy") |
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.
We need this line!
0449df1
to
c6113bf
Compare
PR submitted for Load_Platform to read global variables instead of environment variables: Going to test ACL/LDAP toggling. |
d5b806b
to
97a827b
Compare
c947f66
to
8558f08
Compare
I have tested the following scenarios.
The ADOP LDAP environment variable is set in the adop-docker-compose for Jenkins. This PR can be merged on it's own. |
Conditional steps for LDAP is it modifiable or not.
@SachinKSingh28 I've merged Maris changes so this will need another review. |
As a platform user,
I want to be able to toggle ACL.
This is dependent on
ADOP_ACL_ENABLED
environment variable for adop-docker-compose for Jenkins.Dependent on;
Inject ADOP LDAP and ACL enabled environment variables. adop-jenkins#35Updated Jenkins image to 0.2.5 adop-docker-compose#194