moresandeep commented on code in PR #1239: URL: https://github.com/apache/knox/pull/1239#discussion_r3282698492
########## .github/workflows/compose/docker-compose.yml: ########## @@ -8,32 +8,23 @@ # http://www.apache.org/licenses/LICENSE-2.0 # <p> # Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations under -# the License. +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. services: knox-dev: build: - context: ../build - dockerfile: Dockerfile - image: apache/knox-dev:master - - knox-dev-local: - build: - context: ../build - dockerfile: Dockerfile.local - args: - knoxurl: ${knoxurl:-https://github.com/apache/knox.git} - branch: ${branch:-master} + context: ../../../ Review Comment: Add a `.dockerignore` at the root that allowlists only what the Dockerfile needs: e.g. * !target/*/knox-*.tar.gz !target/*/knoxshell-*.tar.gz !.github/workflows/build/ The build log shows about 200 MB of artifacts being transferred on every build. Looks like every build sends the full Maven target directory, .git history, docs, and any local credentials to the Docker daemon. This is both slow and a potential (I am not sure yet) secrets leakage. ########## .github/workflows/tests.yml: ########## @@ -54,20 +50,11 @@ jobs: -Dshellcheck.skip=true -Dxml.skip=true \ -s .github/workflows/build/settings.xml - - name: Extract Artifacts - run: | - mkdir -p .github/workflows/build/knox-temp-artifacts .github/workflows/build/knoxshell-temp-artifacts - # Extract artifacts to the build directory where Dockerfile expects them - tar -xvzf target/*/knox-*.tar.gz -C .github/workflows/build/knox-temp-artifacts - tar -xvzf target/*/knoxshell-*.tar.gz -C .github/workflows/build/knoxshell-temp-artifacts - - name: Set up Docker Compose run: docker compose version - name: Build Docker Images run: | - export knoxurl=${KNOX_URL} - export branch=${BRANCH:-master} Review Comment: knox-dev service built the official image tagged `apache/knox-dev:master`. Service now builds an image tagged `local-${GITHUB_RUN_ID:-local}-${GITHUB_RUN_ID:-local}`. basically, there is no master tag. This causes two issues, there will be lots of images in the Apache repo which will exhaust our quota, secondly, it is not easy to pickup a knox image will the most latest changes. The idea behind using `apache/knox-dev:master` was to save space and provide folks with an image with all commits on master. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
