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.

Sign up for GitHub

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

Override Default Distribution Download Url with Custom Distribution Url When User Passes a Url #2086

Merged
merged 5 commits into from
Feb 18, 2022
Merged

Override Default Distribution Download Url with Custom Distribution Url When User Passes a Url #2086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1bd18b0
Override Default Distribution Download Url with Custom Distribution U…
Rishikesh1159 Feb 10, 2022
ae4806d
Adding test to check if correct IVY repos were added
Rishikesh1159 Feb 11, 2022
5944093
Adding another test case when custom url is not passed and documentin…
Rishikesh1159 Feb 16, 2022
00d8023
Adding TOC and making changes in DEVELOPER_GUIDE
Rishikesh1159 Feb 16, 2022
17bcb6a
Making changes in DEVELOPER_GUIDE about Distribution Download plugin
Rishikesh1159 Feb 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions DEVELOPER_GUIDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
- [runtimeOnly](#runtimeonly)
- [compileOnly](#compileonly)
- [testImplementation](#testimplementation)
- [Gradle Plugins](#gradle-plugins)
- [Distribution Download Plugin](#distribution-download-plugin)
- [Misc](#misc)
- [git-secrets](#git-secrets)
- [Installation](#installation)
Expand Down Expand Up @@ -340,6 +342,15 @@ somehow. OpenSearch plugins use this configuration to include dependencies that
Code that is on the classpath for compiling tests that are part of this project but not production code. The canonical example
of this is `junit`.

### Gradle Plugins

#### Distribution Download Plugin

The Distribution Download plugin downloads the latest version of OpenSearch by default, and supports overriding this behavior by setting `customDistributionUrl`.
```
./gradlew integTest -DcustomDistributionUrl="https://ci.opensearch.org/ci/dbc/bundle-build/1.2.0/1127/linux/x64/dist/opensearch-1.2.0-linux-x64.tar.gz"
```

## Misc

### git-secrets
Expand Down
28 changes: 17 additions & 11 deletions buildSrc/src/main/java/org/opensearch/gradle/DistributionDownloadPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ public void apply(Project project) {

setupResolutionsContainer(project);
setupDistributionContainer(project, dockerSupport);
setupDownloadServiceRepo(project);
project.afterEvaluate(this::setupDistributions);
}

Expand Down Expand Up @@ -153,6 +152,7 @@ void setupDistributions(Project project) {
dependencies.add(distribution.getExtracted().getName(), distributionDependency.getExtractedNotation());
}
}
setupDownloadServiceRepo(project);
}

private DistributionDependency resolveDependencyNotation(Project p, OpenSearchDistribution distribution) {
Expand Down Expand Up @@ -195,16 +195,22 @@ private static void setupDownloadServiceRepo(Project project) {
if (project.getRepositories().findByName(DOWNLOAD_REPO_NAME) != null) {
return;
}
addIvyRepo(
project,
DOWNLOAD_REPO_NAME,
"https://artifacts.opensearch.org",
FAKE_IVY_GROUP,
"/releases" + RELEASE_PATTERN_LAYOUT,
"/release-candidates" + RELEASE_PATTERN_LAYOUT
);

addIvyRepo(project, SNAPSHOT_REPO_NAME, "https://artifacts.opensearch.org", FAKE_SNAPSHOT_IVY_GROUP, SNAPSHOT_PATTERN_LAYOUT);
Object customDistributionUrl = project.findProperty("customDistributionUrl");
// checks if custom Distribution Url has been passed by user from plugins
if (customDistributionUrl != null) {
addIvyRepo(project, DOWNLOAD_REPO_NAME, customDistributionUrl.toString(), FAKE_IVY_GROUP, "");
addIvyRepo(project, SNAPSHOT_REPO_NAME, customDistributionUrl.toString(), FAKE_SNAPSHOT_IVY_GROUP, "");
} else {
addIvyRepo(
project,
DOWNLOAD_REPO_NAME,
"https://artifacts.opensearch.org",
FAKE_IVY_GROUP,
"/releases" + RELEASE_PATTERN_LAYOUT,
"/release-candidates" + RELEASE_PATTERN_LAYOUT
);
addIvyRepo(project, SNAPSHOT_REPO_NAME, "https://artifacts.opensearch.org", FAKE_SNAPSHOT_IVY_GROUP, SNAPSHOT_PATTERN_LAYOUT);
}

addIvyRepo2(project, DOWNLOAD_REPO_NAME_ES, "https://artifacts-no-kpi.elastic.co", FAKE_IVY_GROUP_ES);
addIvyRepo2(project, SNAPSHOT_REPO_NAME_ES, "https://snapshots-no-kpi.elastic.co", FAKE_SNAPSHOT_IVY_GROUP_ES);
Expand Down
53 changes: 53 additions & 0 deletions buildSrc/src/test/java/org/opensearch/gradle/DistributionDownloadPluginTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.opensearch.gradle.test.GradleUnitTestCase;
import org.gradle.api.NamedDomainObjectContainer;
import org.gradle.api.Project;
import org.gradle.api.internal.artifacts.repositories.DefaultIvyArtifactRepository;
import org.gradle.testfixtures.ProjectBuilder;

import java.io.File;
Expand Down Expand Up @@ -79,6 +80,58 @@ public void testVersionDefault() {
assertEquals(distro.getVersion(), VersionProperties.getOpenSearch());
}

public void testCustomDistributionUrlWithUrl() {
Project project = createProject(null, false);
String customUrl = "https://artifacts.opensearch.org/custom";
project.getExtensions().getExtraProperties().set("customDistributionUrl", customUrl);
DistributionDownloadPlugin plugin = project.getPlugins().getPlugin(DistributionDownloadPlugin.class);
plugin.setupDistributions(project);
assertEquals(4, project.getRepositories().size());
assertEquals(
((DefaultIvyArtifactRepository) project.getRepositories().getAt("opensearch-downloads")).getUrl().toString(),
customUrl
);
assertEquals(
((DefaultIvyArtifactRepository) project.getRepositories().getAt("opensearch-snapshots")).getUrl().toString(),
customUrl
);
assertEquals(
((DefaultIvyArtifactRepository) project.getRepositories().getAt("elasticsearch-downloads")).getUrl().toString(),
"https://artifacts-no-kpi.elastic.co"
);
assertEquals(
((DefaultIvyArtifactRepository) project.getRepositories().getAt("elasticsearch-snapshots")).getUrl().toString(),
"https://snapshots-no-kpi.elastic.co"
);
}

public void testCustomDistributionUrlWithoutUrl() {
Project project = createProject(null, false);
DistributionDownloadPlugin plugin = project.getPlugins().getPlugin(DistributionDownloadPlugin.class);
plugin.setupDistributions(project);
assertEquals(5, project.getRepositories().size());
Copy link
Member

Choose a reason for hiding this comment

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

Why do we still get 5 here? (And check for 4 below)

Copy link
Member Author

@Rishikesh1159 Rishikesh1159 Feb 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

In case of when we not pass customDistribution URL to setupDownloadService() it goes to else block and calls addIvyRepo() with releases and release-candidates as last two parameters for PATTERN LAYOUT. In case when we pass customDistributionUrl PATTERN LAYOUT will be empty.

code: in setupDownloadService() else block

addIvyRepo(
project,
DOWNLOAD_REPO_NAME,
"https://artifacts.opensearch.org",
FAKE_IVY_GROUP,
"/releases" + RELEASE_PATTERN_LAYOUT,
"/release-candidates" + RELEASE_PATTERN_LAYOUT
);

In addIvyRepo() method we have Arrays.stream() which iterates through PATTERN LAYOUT parameter for two times and create two separate repos one for releases and one for release-candidates but both having same url. So, this is why we have 5 repos added instead of 4 for case of not passing custom Distribution url.

code: in addIvyRepo()

private static void addIvyRepo(Project project, String name, String url, String group, String... patternLayout) {
final List repos = Arrays.stream(patternLayout).map(pattern -> project.getRepositories().ivy(repo -> {

Copy link
Member

@dblock dblock Feb 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

My question is more what do we want to do about this? If the URL is the going to be the same all the time, the second one should be removed, shouldn't it? If not, adjust the test for now to check all values.

I hit approve, but consider fixing this before merging.

Copy link
Member Author

@Rishikesh1159 Rishikesh1159 Feb 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes URL will be same both times. But I think either if we want to remove the second repo from repositories set or add a test check(opensearch-downloads2) again for same URL, we need to perform extra operation. Please correct me if I am wrong, adjusting the test to check all values would be like an extra safe step and better than removing repo. So, I will add test (checking opensearch-downloads2) to check all values as you mentioned in your previous suggestion.

Copy link
Member

Choose a reason for hiding this comment

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

works for me.

assertEquals(
((DefaultIvyArtifactRepository) project.getRepositories().getAt("opensearch-downloads")).getUrl().toString(),
"https://artifacts.opensearch.org"
);
assertEquals(
((DefaultIvyArtifactRepository) project.getRepositories().getAt("opensearch-downloads2")).getUrl().toString(),
"https://artifacts.opensearch.org"
);
assertEquals(
((DefaultIvyArtifactRepository) project.getRepositories().getAt("opensearch-snapshots")).getUrl().toString(),
"https://artifacts.opensearch.org"
);
assertEquals(
((DefaultIvyArtifactRepository) project.getRepositories().getAt("elasticsearch-downloads")).getUrl().toString(),
"https://artifacts-no-kpi.elastic.co"
);
assertEquals(
((DefaultIvyArtifactRepository) project.getRepositories().getAt("elasticsearch-snapshots")).getUrl().toString(),
"https://snapshots-no-kpi.elastic.co"
);
}

public void testBadVersionFormat() {
assertDistroError(
createProject(null, false),
Expand Down