Skip to content
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

Use credentials to load GSKit binaries from Artifactory #18666

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

KostasTsiounis
Copy link
Contributor

This allows the GSKit binaries to be downloaded from Artifactory, using the appropriate credentials, during the OpenJDK build process.

Signed-off by: Kostas Tsiounis [email protected]

Copy link
Contributor

@AdamBrousseau AdamBrousseau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're here, is it wise to clean this code up a bit?
I'm wondering if we can move the get_source line to a new function so we don't have to repeat it 4 times. Can a similar thing be done for the gskit creds bit too? 🤔

@KostasTsiounis
Copy link
Contributor Author

KostasTsiounis commented Dec 21, 2023

I can move the whole if-else block into a function to avoid doing it twice. However, I think I need to make separate calls with and without -gskit-credentials. I tried just adding it to the EXTRA_GETSOURCE_OPTIONS variable, or assigning it to its own variable, and then calling the get_source line separately, but even though it works, it seems to be printing the token to the console, which I don't think is the way to go.

@AdamBrousseau If you have a way to circumvent that, I'd be more than willing to give it a try. Otherwise, I can just move the whole new block into a method.

@AdamBrousseau
Copy link
Contributor

Does this work for the get_source line?

if (EXTRA_GETSOURCE_OPTIONS.contains("-gskit-bin") || (EXTRA_CONFIGURE_OPTIONS.contains("-gskit-sdk-bin"))) {
        gskitCredentialID = variableFile.get_user_credentials_id('gskit')
        withCredentials([usernamePassword(credentialsId: "${gskitCredentialID}", passwordVariable: 'GSKIT_PASSWORD', usernameVariable: 'GSKIT_USERNAME')]) {
            get_source_call("-gskit-credential=\$GSKIT_USERNAME:\$GSKIT_PASSWORD")
        }
    } else {
        get_source_call()
    }
def get_source_call(gskit_cred="") {
    sh "bash get_source.sh ${EXTRA_GETSOURCE_OPTIONS} ${gskit_cred} ${OPENJ9_REPO_OPTION} ${OPENJ9_BRANCH_OPTION} ${OPENJ9_SHA_OPTION} ${OPENJ9_REFERENCE} ${OMR_REPO_OPTION} ${OMR_BRANCH_OPTION} ${OMR_SHA_OPTION} ${OMR_REFERENCE}"
}

Or is that what you tried when you said it was printing the creds to the console?

@KostasTsiounis
Copy link
Contributor Author

KostasTsiounis commented Dec 21, 2023

I tried the following:

if (EXTRA_GETSOURCE_OPTIONS.contains("-gskit-bin") || (EXTRA_CONFIGURE_OPTIONS.contains("-gskit-sdk-bin"))) {
        gskitCredentialID = variableFile.get_user_credentials_id('gskit')
        withCredentials([usernamePassword(credentialsId: "${gskitCredentialID}", passwordVariable: 'GSKIT_PASSWORD', usernameVariable: 'GSKIT_USERNAME')]) {
            EXTRA_GETSOURCE_OPTIONS += " -gskit-credential=\$GSKIT_USERNAME:\$GSKIT_PASSWORD"
        }
    } 

sh "bash get_source.sh ${EXTRA_GETSOURCE_OPTIONS} ${OPENJ9_REPO_OPTION} ${OPENJ9_BRANCH_OPTION} ${OPENJ9_SHA_OPTION} ${OPENJ9_REFERENCE} ${OMR_REPO_OPTION} ${OMR_BRANCH_OPTION} ${OMR_SHA_OPTION} ${OMR_REFERENCE}"

Which wouldn't pass the credentials at all. And then I changed the line to:
EXTRA_GETSOURCE_OPTIONS += " -gskit-credential=${GSKIT_USERNAME}:${GSKIT_PASSWORD}"

That worked, but was printing the credentials.

@AdamBrousseau
Copy link
Contributor

Ok. Well let's not waste too much time on it. The code doesn't change that often.

@AdamBrousseau
Copy link
Contributor

jenkins compile amac jdk17

@AdamBrousseau
Copy link
Contributor

jenkins compile amac jdk17

Copy link
Contributor

@AdamBrousseau AdamBrousseau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for getting the code refactored.
PR build passed and internal build passed (Build_JDK17_x86-64_linux_Personal/1825 & 1826).

@AdamBrousseau AdamBrousseau merged commit ecdde62 into eclipse-openj9:master Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants