Make ActionRequests that extend UpdateRequest to extend more generic …

…ActionRequest (#1311) * Upgrade to actions/upload-artifact@v4 Signed-off-by: Craig Perkins <cwperx@amazon.com> * Make ActionRequests that extend UpdateRequest to extend more generic ActionRequest Signed-off-by: Craig Perkins <cwperx@amazon.com> * Apply to StartTransformRequest Signed-off-by: Craig Perkins <cwperx@amazon.com> * Apply to rollup actions Signed-off-by: Craig Perkins <cwperx@amazon.com> * Update snapshot management requests Signed-off-by: Craig Perkins <cwperx@amazon.com> * Fix tests Signed-off-by: Craig Perkins <cwperx@amazon.com> * Free up disk space on github runner Signed-off-by: Craig Perkins <cwperx@amazon.com> * Remove sudo Signed-off-by: Craig Perkins <cwperx@amazon.com> * Check runner space Signed-off-by: Craig Perkins <cwperx@amazon.com> * Remove sudo Signed-off-by: Craig Perkins <cwperx@amazon.com> * Delete index in test Signed-off-by: Craig Perkins <cwperx@amazon.com> * Skip generic Signed-off-by: Craig Perkins <cwperx@amazon.com> * Clear AfterClass Signed-off-by: Craig Perkins <cwperx@amazon.com> * Permissive warnings handler Signed-off-by: Craig Perkins <cwperx@amazon.com> * Move into waitFor Signed-off-by: Craig Perkins <cwperx@amazon.com> * Remove match to generic Signed-off-by: Craig Perkins <cwperx@amazon.com> * Revert "Permissive warnings handler" This reverts commit 7c82470. Signed-off-by: Craig Perkins <cwperx@amazon.com> * Move back out of waitFor Signed-off-by: Craig Perkins <cwperx@amazon.com> * Fix issue with readonly indices after bugfix in core (opensearch-project/OpenSearch#16568) Signed-off-by: Craig Perkins <cwperx@amazon.com> * Overwrite true Signed-off-by: Craig Perkins <cwperx@amazon.com> --------- Signed-off-by: Craig Perkins <cwperx@amazon.com>
opensearch-project · Dec 13, 2024 · 6617cf0 · 6617cf0
1 parent e83ad22
commit 6617cf0
Show file tree

Hide file tree

Showing 19 changed files with 157 additions and 98 deletions.
diff --git 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
@@ -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
@@ -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/...agement/indexstatemanagement/transport/action/removepolicy/TransportRemovePolicyAction.kt b/...agement/indexstatemanagement/transport/action/removepolicy/TransportRemovePolicyAction.kt
@@ -268,17 +268,27 @@ 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),
                         ),
                 )
             }
             if (readOnlyAllowDeleteIndices.isNotEmpty()) {
                 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
@@ -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/...n/kotlin/org/opensearch/indexmanagement/rollup/action/start/TransportStartRollupAction.kt b/...n/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<AcknowledgedResponse>) {
         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<UpdateResponse> {
                 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
@@ -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/...ain/kotlin/org/opensearch/indexmanagement/rollup/action/stop/TransportStopRollupAction.kt b/...ain/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<AcknowledgedResponse>) {
-        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<String>(
                 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<AcknowledgedResponse>) {
         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<UpdateResponse> {
                 override fun onResponse(response: UpdateResponse) {
                     actionListener.onResponse(AcknowledgedResponse(response.result == DocWriteResponse.Result.UPDATED))

diff --git a/...n/org/opensearch/indexmanagement/snapshotmanagement/api/transport/start/StartSMRequest.kt b/...n/org/opensearch/indexmanagement/snapshotmanagement/api/transport/start/StartSMRequest.kt
@@ -5,31 +5,38 @@
 
 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
     }
 
     @Throws(IOException::class)
     override fun writeTo(out: StreamOutput) {
         super.writeTo(out)
+        out.writeString(id)
     }
 }