From 6617cf047e6dab0f61ac9752f2d6bfbca836db1e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 13 Dec 2024 12:15:53 -0500 Subject: [PATCH] Make ActionRequests that extend UpdateRequest to extend more generic ActionRequest (#1311) * Upgrade to actions/upload-artifact@v4 Signed-off-by: Craig Perkins * Make ActionRequests that extend UpdateRequest to extend more generic ActionRequest Signed-off-by: Craig Perkins * Apply to StartTransformRequest Signed-off-by: Craig Perkins * Apply to rollup actions Signed-off-by: Craig Perkins * Update snapshot management requests Signed-off-by: Craig Perkins * Fix tests Signed-off-by: Craig Perkins * Free up disk space on github runner Signed-off-by: Craig Perkins * Remove sudo Signed-off-by: Craig Perkins * Check runner space Signed-off-by: Craig Perkins * Remove sudo Signed-off-by: Craig Perkins * Delete index in test Signed-off-by: Craig Perkins * Skip generic Signed-off-by: Craig Perkins * Clear AfterClass Signed-off-by: Craig Perkins * Permissive warnings handler Signed-off-by: Craig Perkins * Move into waitFor Signed-off-by: Craig Perkins * Remove match to generic Signed-off-by: Craig Perkins * Revert "Permissive warnings handler" This reverts commit 7c824704dd3d06ad5e917a12859fa49647d7b46f. Signed-off-by: Craig Perkins * Move back out of waitFor Signed-off-by: Craig Perkins * Fix issue with readonly indices after bugfix in core (https://github.com/opensearch-project/OpenSearch/pull/16568) Signed-off-by: Craig Perkins * Overwrite true Signed-off-by: Craig Perkins --------- Signed-off-by: Craig Perkins --- .../workflows/multi-node-test-workflow.yml | 13 ++++++------ .github/workflows/security-test-workflow.yml | 13 ++++++------ .github/workflows/test-and-build-workflow.yml | 20 +++++++++--------- .../TransportRemovePolicyAction.kt | 18 ++++++++++++---- .../rollup/action/start/StartRollupRequest.kt | 17 ++++++++++----- .../start/TransportStartRollupAction.kt | 8 ++++--- .../rollup/action/stop/StopRollupRequest.kt | 17 ++++++++++----- .../action/stop/TransportStopRollupAction.kt | 9 ++++---- .../api/transport/start/StartSMRequest.kt | 21 ++++++++++++------- .../transport/start/TransportStartSMAction.kt | 12 ++++++----- .../api/transport/stop/StopSMRequest.kt | 21 ++++++++++++------- .../transport/stop/TransportStopSMAction.kt | 12 ++++++----- .../action/start/StartTransformRequest.kt | 17 ++++++++++----- .../start/TransportStartTransformAction.kt | 7 ++++--- .../action/stop/StopTransformRequest.kt | 17 ++++++++++----- .../stop/TransportStopTransformAction.kt | 9 ++++---- .../rollup/action/RequestTests.kt | 8 +++---- .../snapshotmanagement/action/RequestTests.kt | 8 +++---- .../transform/action/RequestTests.kt | 8 +++---- 19 files changed, 157 insertions(+), 98 deletions(-) diff --git a/.github/workflows/multi-node-test-workflow.yml b/.github/workflows/multi-node-test-workflow.yml index 40d9cc905..02a693ff1 100644 --- a/.github/workflows/multi-node-test-workflow.yml +++ b/.github/workflows/multi-node-test-workflow.yml @@ -7,8 +7,6 @@ on: push: branches: - "**" -env: - ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true jobs: Get-CI-Image-Tag: @@ -35,25 +33,26 @@ jobs: # using the same image which is used by opensearch-build team to build the OpenSearch Distribution # this image tag is subject to change as more dependencies and updates will arrive over time image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }} - # need to switch to root so that github actions can install runner binary on container without permission issues. - options: --user root + options: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-options }} steps: + - name: Run start commands + run: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-command }} # This step uses the setup-java Github action: https://github.com/actions/setup-java - name: Set Up JDK - uses: actions/setup-java@v2 + uses: actions/setup-java@v4 with: distribution: temurin # Temurin is a distribution of adoptium java-version: 21 # index-management - name: Checkout Branch - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Run integration tests with multi node config run: | chown -R 1000:1000 `pwd` su `id -un 1000` -c "./gradlew integTest -PnumNodes=3 ${{ env.TEST_FILTER }}" - name: Upload failed logs - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 if: failure() with: name: logs diff --git a/.github/workflows/security-test-workflow.yml b/.github/workflows/security-test-workflow.yml index 8dc6eb19c..6b0363fc7 100644 --- a/.github/workflows/security-test-workflow.yml +++ b/.github/workflows/security-test-workflow.yml @@ -7,8 +7,6 @@ on: push: branches: - "**" -env: - ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true jobs: Get-CI-Image-Tag: @@ -24,25 +22,26 @@ jobs: # using the same image which is used by opensearch-build team to build the OpenSearch Distribution # this image tag is subject to change as more dependencies and updates will arrive over time image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }} - # need to switch to root so that github actions can install runner binary on container without permission issues. - options: --user root + options: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-options }} steps: + - name: Run start commands + run: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-command }} # This step uses the setup-java Github action: https://github.com/actions/setup-java - name: Set Up JDK - uses: actions/setup-java@v2 + uses: actions/setup-java@v4 with: distribution: temurin # Temurin is a distribution of adoptium java-version: 21 # index-management - name: Checkout Branch - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Run integration tests run: | chown -R 1000:1000 `pwd` su `id -un 1000` -c "./gradlew integTest -Dsecurity=true -Dhttps=true --tests '*IT'" - name: Upload failed logs - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 if: failure() with: name: logs diff --git a/.github/workflows/test-and-build-workflow.yml b/.github/workflows/test-and-build-workflow.yml index 5713aea5e..32870eefa 100644 --- a/.github/workflows/test-and-build-workflow.yml +++ b/.github/workflows/test-and-build-workflow.yml @@ -6,8 +6,6 @@ on: push: branches: - "**" -env: - ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true jobs: Get-CI-Image-Tag: @@ -36,19 +34,20 @@ jobs: # using the same image which is used by opensearch-build team to build the OpenSearch Distribution # this image tag is subject to change as more dependencies and updates will arrive over time image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }} - # need to switch to root so that github actions can install runner binary on container without permission issues. - options: --user root + options: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-options }} steps: + - name: Run start commands + run: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-command }} # This step uses the setup-java Github action: https://github.com/actions/setup-java - name: Set Up JDK ${{ matrix.java }} - uses: actions/setup-java@v2 + uses: actions/setup-java@v4 with: distribution: temurin # Temurin is a distribution of adoptium java-version: ${{ matrix.java }} # build index management - name: Checkout Branch - uses: actions/checkout@v2 + uses: actions/checkout@v4 # This is a hack, but this step creates a link to the X: mounted drive, which makes the path # short enough to work on Windows - name: Build with Gradle @@ -61,18 +60,19 @@ jobs: with: name: logs-${{ matrix.java }}-${{ matrix.feature }} path: build/testclusters/integTest-*/logs/* + overwrite: 'true' - name: Create Artifact Path run: | mkdir -p index-management-artifacts cp ./build/distributions/*.zip index-management-artifacts - name: Uploads coverage - uses: codecov/codecov-action@v1 + uses: codecov/codecov-action@v4 with: token: ${{ secrets.CODECOV_TOKEN }} # This step uses the upload-artifact Github action: https://github.com/actions/upload-artifact - name: Upload Artifacts # v4 requires node.js 20 which is not supported - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: index-management-plugin-ubuntu-latest-${{ matrix.java }} path: index-management-artifacts @@ -106,13 +106,13 @@ jobs: steps: # This step uses the setup-java Github action: https://github.com/actions/setup-java - name: Set Up JDK ${{ matrix.java }} - uses: actions/setup-java@v2 + uses: actions/setup-java@v4 with: distribution: temurin # Temurin is a distribution of adoptium java-version: ${{ matrix.java }} # build index management - name: Checkout Branch - uses: actions/checkout@v2 + uses: actions/checkout@v4 # This is a hack, but this step creates a link to the X: mounted drive, which makes the path # short enough to work on Windows - name: Shorten Path diff --git a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/removepolicy/TransportRemovePolicyAction.kt b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/removepolicy/TransportRemovePolicyAction.kt index dcefa694e..55150225e 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/removepolicy/TransportRemovePolicyAction.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/removepolicy/TransportRemovePolicyAction.kt @@ -268,8 +268,13 @@ constructor( updateSettingReqsList.add( UpdateSettingsRequest().indices(*readOnlyIndices.map { indices[it] }.toTypedArray()) .settings( - Settings.builder().put(ManagedIndexSettings.AUTO_MANAGE.key, false) - .put(INDEX_READ_ONLY_SETTING.key, true), + Settings.builder().put(INDEX_READ_ONLY_SETTING.key, false), + ), + ) + updateSettingReqsList.add( + UpdateSettingsRequest().indices(*readOnlyIndices.map { indices[it] }.toTypedArray()) + .settings( + Settings.builder().put(ManagedIndexSettings.AUTO_MANAGE.key, false).put(INDEX_READ_ONLY_SETTING.key, true), ), ) } @@ -277,8 +282,13 @@ constructor( updateSettingReqsList.add( UpdateSettingsRequest().indices(*readOnlyAllowDeleteIndices.map { indices[it] }.toTypedArray()) .settings( - Settings.builder().put(ManagedIndexSettings.AUTO_MANAGE.key, false) - .put(INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.key, true), + Settings.builder().put(INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.key, false), + ), + ) + updateSettingReqsList.add( + UpdateSettingsRequest().indices(*readOnlyAllowDeleteIndices.map { indices[it] }.toTypedArray()) + .settings( + Settings.builder().put(ManagedIndexSettings.AUTO_MANAGE.key, false).put(INDEX_BLOCKS_READ_ONLY_ALLOW_DELETE_SETTING.key, true), ), ) } diff --git a/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/start/StartRollupRequest.kt b/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/start/StartRollupRequest.kt index 3493d5473..5d4e63b36 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/start/StartRollupRequest.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/start/StartRollupRequest.kt @@ -5,24 +5,30 @@ package org.opensearch.indexmanagement.rollup.action.start +import org.opensearch.action.ActionRequest import org.opensearch.action.ActionRequestValidationException import org.opensearch.action.ValidateActions.addValidationError -import org.opensearch.action.update.UpdateRequest import org.opensearch.core.common.io.stream.StreamInput import org.opensearch.core.common.io.stream.StreamOutput import java.io.IOException -class StartRollupRequest : UpdateRequest { +class StartRollupRequest : ActionRequest { + + val id: String + get() = field + @Throws(IOException::class) - constructor(sin: StreamInput) : super(sin) + constructor(sin: StreamInput) : super(sin) { + this.id = sin.readString() + } constructor(id: String) { - super.id(id) + this.id = id } override fun validate(): ActionRequestValidationException? { var validationException: ActionRequestValidationException? = null - if (super.id().isEmpty()) { + if (this.id.isEmpty()) { validationException = addValidationError("id is missing", validationException) } return validationException @@ -31,5 +37,6 @@ class StartRollupRequest : UpdateRequest { @Throws(IOException::class) override fun writeTo(out: StreamOutput) { super.writeTo(out) + out.writeString(id) } } diff --git a/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/start/TransportStartRollupAction.kt b/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/start/TransportStartRollupAction.kt index 535c834b5..8689dc221 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/start/TransportStartRollupAction.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/start/TransportStartRollupAction.kt @@ -28,6 +28,7 @@ import org.opensearch.commons.authuser.User import org.opensearch.core.action.ActionListener import org.opensearch.core.rest.RestStatus import org.opensearch.core.xcontent.NamedXContentRegistry +import org.opensearch.indexmanagement.IndexManagementPlugin import org.opensearch.indexmanagement.IndexManagementPlugin.Companion.INDEX_MANAGEMENT_INDEX import org.opensearch.indexmanagement.opensearchapi.parseWithType import org.opensearch.indexmanagement.rollup.model.Rollup @@ -70,7 +71,7 @@ constructor( ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, )}", ) - val getReq = GetRequest(INDEX_MANAGEMENT_INDEX, request.id()) + val getReq = GetRequest(INDEX_MANAGEMENT_INDEX, request.id) val user: User? = buildUser(client.threadPool().threadContext) client.threadPool().threadContext.stashContext().use { client.get( @@ -115,7 +116,8 @@ constructor( // TODO: Should create a transport action to update metadata private fun updateRollupJob(rollup: Rollup, request: StartRollupRequest, actionListener: ActionListener) { val now = Instant.now().toEpochMilli() - request.index(INDEX_MANAGEMENT_INDEX).doc( + val updateReq = UpdateRequest(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX, request.id) + updateReq.doc( mapOf( Rollup.ROLLUP_TYPE to mapOf( @@ -125,7 +127,7 @@ constructor( ), ) client.update( - request, + updateReq, object : ActionListener { override fun onResponse(response: UpdateResponse) { if (response.result == DocWriteResponse.Result.UPDATED) { diff --git a/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/stop/StopRollupRequest.kt b/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/stop/StopRollupRequest.kt index 1a03317a7..0b5d4b75d 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/stop/StopRollupRequest.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/stop/StopRollupRequest.kt @@ -5,24 +5,30 @@ package org.opensearch.indexmanagement.rollup.action.stop +import org.opensearch.action.ActionRequest import org.opensearch.action.ActionRequestValidationException import org.opensearch.action.ValidateActions.addValidationError -import org.opensearch.action.update.UpdateRequest import org.opensearch.core.common.io.stream.StreamInput import org.opensearch.core.common.io.stream.StreamOutput import java.io.IOException -class StopRollupRequest : UpdateRequest { +class StopRollupRequest : ActionRequest { + + val id: String + get() = field + @Throws(IOException::class) - constructor(sin: StreamInput) : super(sin) + constructor(sin: StreamInput) : super(sin) { + this.id = sin.readString() + } constructor(id: String) { - super.id(id) + this.id = id } override fun validate(): ActionRequestValidationException? { var validationException: ActionRequestValidationException? = null - if (super.id().isEmpty()) { + if (this.id.isEmpty()) { validationException = addValidationError("id is missing", validationException) } return validationException @@ -31,5 +37,6 @@ class StopRollupRequest : UpdateRequest { @Throws(IOException::class) override fun writeTo(out: StreamOutput) { super.writeTo(out) + out.writeString(id) } } diff --git a/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/stop/TransportStopRollupAction.kt b/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/stop/TransportStopRollupAction.kt index fdeead257..8aad70107 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/stop/TransportStopRollupAction.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/rollup/action/stop/TransportStopRollupAction.kt @@ -77,13 +77,13 @@ constructor( @Suppress("ReturnCount") override fun doExecute(task: Task, request: StopRollupRequest, actionListener: ActionListener) { - log.debug("Executing StopRollupAction on ${request.id()}") + log.debug("Executing StopRollupAction on ${request.id}") log.debug( "User and roles string from thread context: ${client.threadPool().threadContext.getTransient( ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, )}", ) - val getRequest = GetRequest(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX, request.id()) + val getRequest = GetRequest(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX, request.id) val user = buildUser(client.threadPool().threadContext) client.threadPool().threadContext.stashContext().use { client.get( @@ -214,7 +214,8 @@ constructor( private fun updateRollupJob(rollup: Rollup, request: StopRollupRequest, actionListener: ActionListener) { val now = Instant.now().toEpochMilli() - request.index(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX).setIfSeqNo(rollup.seqNo).setIfPrimaryTerm(rollup.primaryTerm) + val updateReq = UpdateRequest(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX, request.id) + updateReq.setIfSeqNo(rollup.seqNo).setIfPrimaryTerm(rollup.primaryTerm) .doc( mapOf( Rollup.ROLLUP_TYPE to @@ -226,7 +227,7 @@ constructor( ) .routing(rollup.id) client.update( - request, + updateReq, object : ActionListener { override fun onResponse(response: UpdateResponse) { actionListener.onResponse(AcknowledgedResponse(response.result == DocWriteResponse.Result.UPDATED)) diff --git a/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/start/StartSMRequest.kt b/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/start/StartSMRequest.kt index 23e00e16b..6b22dd99e 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/start/StartSMRequest.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/start/StartSMRequest.kt @@ -5,25 +5,31 @@ package org.opensearch.indexmanagement.snapshotmanagement.api.transport.start +import org.opensearch.action.ActionRequest import org.opensearch.action.ActionRequestValidationException -import org.opensearch.action.ValidateActions -import org.opensearch.action.update.UpdateRequest +import org.opensearch.action.ValidateActions.addValidationError import org.opensearch.core.common.io.stream.StreamInput import org.opensearch.core.common.io.stream.StreamOutput import java.io.IOException -class StartSMRequest : UpdateRequest { +class StartSMRequest : ActionRequest { + + val id: String + get() = field + @Throws(IOException::class) - constructor(sin: StreamInput) : super(sin) + constructor(sin: StreamInput) : super(sin) { + this.id = sin.readString() + } constructor(id: String) { - super.id(id) + this.id = id } override fun validate(): ActionRequestValidationException? { var validationException: ActionRequestValidationException? = null - if (super.id().isEmpty()) { - validationException = ValidateActions.addValidationError("id is missing", validationException) + if (this.id.isEmpty()) { + validationException = addValidationError("id is missing", validationException) } return validationException } @@ -31,5 +37,6 @@ class StartSMRequest : UpdateRequest { @Throws(IOException::class) override fun writeTo(out: StreamOutput) { super.writeTo(out) + out.writeString(id) } } diff --git a/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/start/TransportStartSMAction.kt b/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/start/TransportStartSMAction.kt index ac5e58ab5..6e81bb6b7 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/start/TransportStartSMAction.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/start/TransportStartSMAction.kt @@ -11,6 +11,7 @@ import org.opensearch.OpenSearchStatusException import org.opensearch.action.DocWriteResponse import org.opensearch.action.support.ActionFilters import org.opensearch.action.support.master.AcknowledgedResponse +import org.opensearch.action.update.UpdateRequest import org.opensearch.action.update.UpdateResponse import org.opensearch.client.Client import org.opensearch.cluster.service.ClusterService @@ -57,7 +58,7 @@ constructor( user: User?, threadContext: ThreadContext.StoredContext, ): AcknowledgedResponse { - val smPolicy = client.getSMPolicy(request.id()) + val smPolicy = client.getSMPolicy(request.id) // Check if the requested user has permission on the resource, throwing an exception if the user does not verifyUserHasPermissionForResource(user, smPolicy.user, filterByEnabled, "snapshot management policy", smPolicy.policyName) @@ -71,7 +72,8 @@ constructor( private suspend fun enableSMPolicy(updateRequest: StartSMRequest): Boolean { val now = Instant.now().toEpochMilli() - updateRequest.index(INDEX_MANAGEMENT_INDEX).doc( + val updateReq = UpdateRequest(INDEX_MANAGEMENT_INDEX, updateRequest.id) + updateReq.doc( mapOf( SMPolicy.SM_TYPE to mapOf( @@ -83,12 +85,12 @@ constructor( ) val updateResponse: UpdateResponse = try { - client.suspendUntil { update(updateRequest, it) } + client.suspendUntil { update(updateReq, it) } } catch (e: VersionConflictEngineException) { - log.error("VersionConflictEngineException while trying to enable snapshot management policy id [${updateRequest.id()}]: $e") + log.error("VersionConflictEngineException while trying to enable snapshot management policy id [${updateRequest.id}]: $e") throw OpenSearchStatusException(conflictExceptionMessage, RestStatus.INTERNAL_SERVER_ERROR) } catch (e: Exception) { - log.error("Failed trying to enable snapshot management policy id [${updateRequest.id()}]: $e") + log.error("Failed trying to enable snapshot management policy id [${updateRequest.id}]: $e") throw OpenSearchStatusException("Failed while trying to enable SM Policy", RestStatus.INTERNAL_SERVER_ERROR) } return updateResponse.result == DocWriteResponse.Result.UPDATED diff --git a/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/stop/StopSMRequest.kt b/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/stop/StopSMRequest.kt index 4d70096b6..1b7961e09 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/stop/StopSMRequest.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/stop/StopSMRequest.kt @@ -5,25 +5,31 @@ package org.opensearch.indexmanagement.snapshotmanagement.api.transport.stop +import org.opensearch.action.ActionRequest import org.opensearch.action.ActionRequestValidationException -import org.opensearch.action.ValidateActions -import org.opensearch.action.update.UpdateRequest +import org.opensearch.action.ValidateActions.addValidationError import org.opensearch.core.common.io.stream.StreamInput import org.opensearch.core.common.io.stream.StreamOutput import java.io.IOException -class StopSMRequest : UpdateRequest { +class StopSMRequest : ActionRequest { + + val id: String + get() = field + @Throws(IOException::class) - constructor(sin: StreamInput) : super(sin) + constructor(sin: StreamInput) : super(sin) { + this.id = sin.readString() + } constructor(id: String) { - super.id(id) + this.id = id } override fun validate(): ActionRequestValidationException? { var validationException: ActionRequestValidationException? = null - if (super.id().isEmpty()) { - validationException = ValidateActions.addValidationError("id is missing", validationException) + if (this.id.isEmpty()) { + validationException = addValidationError("id is missing", validationException) } return validationException } @@ -31,5 +37,6 @@ class StopSMRequest : UpdateRequest { @Throws(IOException::class) override fun writeTo(out: StreamOutput) { super.writeTo(out) + out.writeString(id) } } diff --git a/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/stop/TransportStopSMAction.kt b/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/stop/TransportStopSMAction.kt index 4e4b0b7e6..6d45735d6 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/stop/TransportStopSMAction.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/snapshotmanagement/api/transport/stop/TransportStopSMAction.kt @@ -11,6 +11,7 @@ import org.opensearch.OpenSearchStatusException import org.opensearch.action.DocWriteResponse import org.opensearch.action.support.ActionFilters import org.opensearch.action.support.master.AcknowledgedResponse +import org.opensearch.action.update.UpdateRequest import org.opensearch.action.update.UpdateResponse import org.opensearch.client.Client import org.opensearch.cluster.service.ClusterService @@ -57,7 +58,7 @@ constructor( user: User?, threadContext: ThreadContext.StoredContext, ): AcknowledgedResponse { - val smPolicy = client.getSMPolicy(request.id()) + val smPolicy = client.getSMPolicy(request.id) // Check if the requested user has permission on the resource, throwing an exception if the user does not verifyUserHasPermissionForResource(user, smPolicy.user, filterByEnabled, "snapshot management policy", smPolicy.policyName) @@ -71,7 +72,8 @@ constructor( private suspend fun disableSMPolicy(updateRequest: StopSMRequest): Boolean { val now = Instant.now().toEpochMilli() - updateRequest.index(INDEX_MANAGEMENT_INDEX).doc( + val updateReq = UpdateRequest(INDEX_MANAGEMENT_INDEX, updateRequest.id) + updateReq.doc( mapOf( SMPolicy.SM_TYPE to mapOf( @@ -83,12 +85,12 @@ constructor( ) val updateResponse: UpdateResponse = try { - client.suspendUntil { update(updateRequest, it) } + client.suspendUntil { update(updateReq, it) } } catch (e: VersionConflictEngineException) { - log.error("VersionConflictEngineException while trying to disable snapshot management policy id [${updateRequest.id()}]: $e") + log.error("VersionConflictEngineException while trying to disable snapshot management policy id [${updateRequest.id}]: $e") throw OpenSearchStatusException(conflictExceptionMessage, RestStatus.INTERNAL_SERVER_ERROR) } catch (e: Exception) { - log.error("Failed trying to disable snapshot management policy id [${updateRequest.id()}]: $e") + log.error("Failed trying to disable snapshot management policy id [${updateRequest.id}]: $e") throw OpenSearchStatusException("Failed while trying to disable SM Policy", RestStatus.INTERNAL_SERVER_ERROR) } // TODO update metadata diff --git a/src/main/kotlin/org/opensearch/indexmanagement/transform/action/start/StartTransformRequest.kt b/src/main/kotlin/org/opensearch/indexmanagement/transform/action/start/StartTransformRequest.kt index a6514c6df..b9335c55c 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/transform/action/start/StartTransformRequest.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/transform/action/start/StartTransformRequest.kt @@ -5,24 +5,30 @@ package org.opensearch.indexmanagement.transform.action.start +import org.opensearch.action.ActionRequest import org.opensearch.action.ActionRequestValidationException import org.opensearch.action.ValidateActions.addValidationError -import org.opensearch.action.update.UpdateRequest import org.opensearch.core.common.io.stream.StreamInput import org.opensearch.core.common.io.stream.StreamOutput import java.io.IOException -class StartTransformRequest : UpdateRequest { +class StartTransformRequest : ActionRequest { + + val id: String + get() = field + @Throws(IOException::class) - constructor(sin: StreamInput) : super(sin) + constructor(sin: StreamInput) : super(sin) { + this.id = sin.readString() + } constructor(id: String) { - super.id(id) + this.id = id } override fun validate(): ActionRequestValidationException? { var validationException: ActionRequestValidationException? = null - if (super.id().isEmpty()) { + if (this.id.isEmpty()) { validationException = addValidationError("id is missing", validationException) } return validationException @@ -31,5 +37,6 @@ class StartTransformRequest : UpdateRequest { @Throws(IOException::class) override fun writeTo(out: StreamOutput) { super.writeTo(out) + out.writeString(id) } } diff --git a/src/main/kotlin/org/opensearch/indexmanagement/transform/action/start/TransportStartTransformAction.kt b/src/main/kotlin/org/opensearch/indexmanagement/transform/action/start/TransportStartTransformAction.kt index b69187d89..3a2c21044 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/transform/action/start/TransportStartTransformAction.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/transform/action/start/TransportStartTransformAction.kt @@ -68,7 +68,7 @@ constructor( ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, )}", ) - val getRequest = GetRequest(INDEX_MANAGEMENT_INDEX, request.id()) + val getRequest = GetRequest(INDEX_MANAGEMENT_INDEX, request.id) val user = buildUser(client.threadPool().threadContext) client.threadPool().threadContext.stashContext().use { client.get( @@ -117,7 +117,8 @@ constructor( actionListener: ActionListener, ) { val now = Instant.now().toEpochMilli() - request.index(INDEX_MANAGEMENT_INDEX).doc( + val updateReq = UpdateRequest(INDEX_MANAGEMENT_INDEX, request.id) + updateReq.doc( mapOf( Transform.TRANSFORM_TYPE to mapOf( @@ -127,7 +128,7 @@ constructor( ), ) client.update( - request, + updateReq, object : ActionListener { override fun onResponse(response: UpdateResponse) { if (response.result == DocWriteResponse.Result.UPDATED) { diff --git a/src/main/kotlin/org/opensearch/indexmanagement/transform/action/stop/StopTransformRequest.kt b/src/main/kotlin/org/opensearch/indexmanagement/transform/action/stop/StopTransformRequest.kt index 7d3a3c2fc..5174abda3 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/transform/action/stop/StopTransformRequest.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/transform/action/stop/StopTransformRequest.kt @@ -5,24 +5,30 @@ package org.opensearch.indexmanagement.transform.action.stop +import org.opensearch.action.ActionRequest import org.opensearch.action.ActionRequestValidationException import org.opensearch.action.ValidateActions.addValidationError -import org.opensearch.action.update.UpdateRequest import org.opensearch.core.common.io.stream.StreamInput import org.opensearch.core.common.io.stream.StreamOutput import java.io.IOException -class StopTransformRequest : UpdateRequest { +class StopTransformRequest : ActionRequest { + + val id: String + get() = field + @Throws(IOException::class) - constructor(sin: StreamInput) : super(sin) + constructor(sin: StreamInput) : super(sin) { + this.id = sin.readString() + } constructor(id: String) { - super.id(id) + this.id = id } override fun validate(): ActionRequestValidationException? { var validationException: ActionRequestValidationException? = null - if (super.id().isEmpty()) { + if (this.id.isEmpty()) { validationException = addValidationError("id is missing", validationException) } return validationException @@ -31,5 +37,6 @@ class StopTransformRequest : UpdateRequest { @Throws(IOException::class) override fun writeTo(out: StreamOutput) { super.writeTo(out) + out.writeString(id) } } diff --git a/src/main/kotlin/org/opensearch/indexmanagement/transform/action/stop/TransportStopTransformAction.kt b/src/main/kotlin/org/opensearch/indexmanagement/transform/action/stop/TransportStopTransformAction.kt index 4e0e480cb..c2b5636d6 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/transform/action/stop/TransportStopTransformAction.kt +++ b/src/main/kotlin/org/opensearch/indexmanagement/transform/action/stop/TransportStopTransformAction.kt @@ -76,13 +76,13 @@ constructor( private val log = LogManager.getLogger(javaClass) override fun doExecute(task: Task, request: StopTransformRequest, actionListener: ActionListener) { - log.debug("Executing StopTransformAction on ${request.id()}") + log.debug("Executing StopTransformAction on ${request.id}") log.debug( "User and roles string from thread context: ${client.threadPool().threadContext.getTransient( ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, )}", ) - val getRequest = GetRequest(INDEX_MANAGEMENT_INDEX, request.id()) + val getRequest = GetRequest(INDEX_MANAGEMENT_INDEX, request.id) val user = buildUser(client.threadPool().threadContext) client.threadPool().threadContext.stashContext().use { client.get( @@ -212,7 +212,8 @@ constructor( private fun updateTransformJob(transform: Transform, request: StopTransformRequest, actionListener: ActionListener) { val now = Instant.now().toEpochMilli() - request.index(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX).setIfSeqNo(transform.seqNo).setIfPrimaryTerm(transform.primaryTerm) + val updateReq = UpdateRequest(IndexManagementPlugin.INDEX_MANAGEMENT_INDEX, request.id) + updateReq.setIfSeqNo(transform.seqNo).setIfPrimaryTerm(transform.primaryTerm) .doc( mapOf( Transform.TRANSFORM_TYPE to @@ -223,7 +224,7 @@ constructor( ), ) client.update( - request, + updateReq, object : ActionListener { override fun onResponse(response: UpdateResponse) { actionListener.onResponse(AcknowledgedResponse(response.result == DocWriteResponse.Result.UPDATED)) diff --git a/src/test/kotlin/org/opensearch/indexmanagement/rollup/action/RequestTests.kt b/src/test/kotlin/org/opensearch/indexmanagement/rollup/action/RequestTests.kt index 3786a20f2..4b880336a 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/rollup/action/RequestTests.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/rollup/action/RequestTests.kt @@ -134,21 +134,21 @@ class RequestTests : OpenSearchTestCase() { fun `test start rollup request`() { val id = "some_id" - val req = StartRollupRequest(id).index(INDEX_MANAGEMENT_INDEX) + val req = StartRollupRequest(id) val out = BytesStreamOutput().apply { req.writeTo(this) } val sin = StreamInput.wrap(out.bytes().toBytesRef().bytes) val streamedReq = StartRollupRequest(sin) - assertEquals(id, streamedReq.id()) + assertEquals(id, streamedReq.id) } fun `test stop rollup request`() { val id = "some_id" - val req = StopRollupRequest(id).index(INDEX_MANAGEMENT_INDEX) + val req = StopRollupRequest(id) val out = BytesStreamOutput().apply { req.writeTo(this) } val sin = StreamInput.wrap(out.bytes().toBytesRef().bytes) val streamedReq = StopRollupRequest(sin) - assertEquals(id, streamedReq.id()) + assertEquals(id, streamedReq.id) } } diff --git a/src/test/kotlin/org/opensearch/indexmanagement/snapshotmanagement/action/RequestTests.kt b/src/test/kotlin/org/opensearch/indexmanagement/snapshotmanagement/action/RequestTests.kt index 440aca087..dbf7016ea 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/snapshotmanagement/action/RequestTests.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/snapshotmanagement/action/RequestTests.kt @@ -82,22 +82,22 @@ class RequestTests : OpenSearchTestCase() { fun `test start sm policy request`() { val id = "some_id" - val req = StartSMRequest(id).index(INDEX_MANAGEMENT_INDEX) + val req = StartSMRequest(id) val out = BytesStreamOutput().apply { req.writeTo(this) } val sin = StreamInput.wrap(out.bytes().toBytesRef().bytes) val streamedReq = StartSMRequest(sin) - assertEquals(id, streamedReq.id()) + assertEquals(id, streamedReq.id) } fun `test stop sm policy request`() { val id = "some_id" - val req = StopSMRequest(id).index(INDEX_MANAGEMENT_INDEX) + val req = StopSMRequest(id) val out = BytesStreamOutput().apply { req.writeTo(this) } val sin = StreamInput.wrap(out.bytes().toBytesRef().bytes) val streamedReq = StopSMRequest(sin) - assertEquals(id, streamedReq.id()) + assertEquals(id, streamedReq.id) } fun `test explain sm policy request`() { diff --git a/src/test/kotlin/org/opensearch/indexmanagement/transform/action/RequestTests.kt b/src/test/kotlin/org/opensearch/indexmanagement/transform/action/RequestTests.kt index e2166ccf1..79456e54d 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/transform/action/RequestTests.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/transform/action/RequestTests.kt @@ -151,21 +151,21 @@ class RequestTests : OpenSearchTestCase() { fun `test start transform request`() { val id = "some_id" - val req = StartTransformRequest(id).index(INDEX_MANAGEMENT_INDEX) + val req = StartTransformRequest(id) val out = BytesStreamOutput().apply { req.writeTo(this) } val streamedReq = StartTransformRequest(buildStreamInputForTransforms(out)) - assertEquals(id, streamedReq.id()) + assertEquals(id, streamedReq.id) } fun `test stop transform request`() { val id = "some_id" - val req = StopTransformRequest(id).index(INDEX_MANAGEMENT_INDEX) + val req = StopTransformRequest(id) val out = BytesStreamOutput().apply { req.writeTo(this) } val streamedReq = StopTransformRequest(buildStreamInputForTransforms(out)) - assertEquals(id, streamedReq.id()) + assertEquals(id, streamedReq.id) } }