Fix issues with docker-compose #41
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- required changes in Dockerfile around yum install for certain openjdk version - required skipping tests when building app Docker image because unit tests are mixed with integration tests that rely on Dockerized services - fixed missinf fat jar functionality with UI as a dependency for API
JohnKallies
requested review from
KaranSinghWgu
and removed request for
drey-bigney
drey-bigney
suggested changes
Sep 21, 2021
| @@ -6,7 +6,6 @@ FROM centos:centos8.3.2011 as osmt-base | |||
| LABEL Maintainer="WGU / OSN" | |||
| LABEL Version="1.0" | |||
|
|
|||
| ENV JAVA_VERSION=11.0.9.11 | |||
There was a problem hiding this comment.
Why are we removing the Java version?
There was a problem hiding this comment.
We continue to pull in the latest OpenJDK 11. yum install is no longer able to find an openjdk package as the name is composed (using a specific version number, line 18, below).
For what it's worth, this isn't an automation issue or Docker networking. I built the image to that point and tried to yum install from the shell in a container. I see the package referenced online, but it's not available in this context.
We could possibly fix this by pointing to a different package repo?
There was a problem hiding this comment.
Are we able to bump the Java version to the latest package?
There was a problem hiding this comment.
I can look at that. I thought that's what we would get by just saying java-11-openjdk on line 17.
There was a problem hiding this comment.
@drey-bigney We were talking offline about pinning to a specific openjdk version. If we pin to specific versions of OS packages or software dependencies, then we absorb responsibility for doing security updates out-of-band from the build process. We trade security updates for the safety of known-good dependency trees. The world keeps turning, and things will drift... Better than pinning to a given version of OpenJDK, we should probably have a pipeline that runs all this and shows a build status badge.
I've added Issue #44 "Implement build badges for docker-compose". To me, this is a superior approach to pinning to a specific patch version of java-11-openjdk, and satisfies the open issues for this PR.
Re: omitting the integration tests from the Docker build, using a Maven profile was the cleanest, duct-tape-free self-documenting approach that I saw.
There was a problem hiding this comment.
@JohnKallies that makes sense. As long as we are sticking with the same major version of the JDK I doubt we'll see breaking changes and this'll free us from breaking the build system every time a minor JDK revision happens.
+1
There was a problem hiding this comment.
Interesting. I thought that patch management was a key responsibility of any project's maintainers... Therefore, pinning is implied by the responsibility to maintain. If you don't pin, you don't know what you're getting. "Trusting the source" has led to countless problems for my team, when they don't follow semver and break compatibility, or the fixes have breaking side-effects that didn't affect the mainline users. I think pinning is the safe approach.
- excluded defacto integration tests from Dockerfile mvn package with "dockerfile-build" Maven profile
hikr17
requested review from
stirhale-wgu
and removed request for
KaranSinghWgu
|
Hmm. I tried to add Stirling as a reviewer, but I think I stomped on the name of another reviewer, but cannot tell which one. I apologize, John. |
drey-bigney
approved these changes
Sep 24, 2021
JohnKallies
added a commit
that referenced
this pull request
Oct 9, 2021
…skills and collections (#16) * Support anonymous API usage of searching and listing for skills or collections. allow for rate limiting public requests with bucket4j * added a clarifying statement under attribution (#17) as per legal department, I have added the statement under attribution about the contribution covenant. * Update CONTRIBUTING.md (#22) Updated contribution guidelines * DMND-631 - Update code to include BLS JobCodes when the skills are im… (#26) * DMND-631 - Update code to include BLS JobCodes when the skills are imported using the UI batch import process. * DMND-631 - Addressed PR comments. * DMND-673 Update open source repo with osmt tests (#29) * DMND-674 Update open source repo with osmt-ui tests (#31) * Test improvements (#32) * * Added more tests * Upgraded testing dependencies to support code coverage. * Missed 2 files. * * Removed isAuthorEditable() in many cases because the intent was separate from using the defaultAuthorValue, but was not coded separately. i.e., if there is a defaultAuthorValue, then use it as the default. (#33) * Fixed the assumption that the same web server is used for both UI and Service. That's not always the case. * Temporary handling of the ElasticSearch limitation of 10000 RSDs. Until that is fixed by other means, we add a "+" to reduce confusion about the 10000 RSD count. * Add noauth profile config for local development (#34) * Add noauth profile config for local development SecurityConfigNoAuth.kt taken from ngp-aa-osmt * Add noauth profile config for local development * Update readme (#9) * added architectural diagram and BLS Onet import process * added Okta steps for setup * inserted PNG to assests folder to use in README file for the architectural diagram * fixed png so that it removed the box with the osmt description. * Closes #18 - Update README for community contributors (#39) * Closes #18 - Update README for community contributors also updated the API module's README * Address review feedback for project README and docs - also made a small tweak to .gitignore to support multiple env files. * Readme formatting * Add Testing Expections to CONTRIBUTING.md (#43) * Add Testing Expections to CONTRIBUTING.md - clarified the OSMT definition of unit test / integration test / e2e test * Update CONTRIBUTING.md Co-authored-by: drey-bigney <[email protected]> * Adding test for ElasticSearchReindexer (#45) * Fix issues with docker-compose (#41) * Fix issues with docker-compose - required changes in Dockerfile around yum install for certain openjdk version - required skipping tests when building app Docker image because unit tests are mixed with integration tests that rely on Dockerized services - fixed missinf fat jar functionality with UI as a dependency for API * Update pom.xml * Aadjust Dockerfile for review requests - excluded defacto integration tests from Dockerfile mvn package with "dockerfile-build" Maven profile * add new integration test to pom file for exclusions in docker build * Feature/add code coverage (#46) * * Added more tests * Upgraded testing dependencies to support code coverage. * Missed 2 files. * * Added more tests. * Added test for RichSkillSearchResultsComponent. Also added support for ActivatedRouteStub.setQueryParams. * Renamed ActivatedRouteStub.setParamsMap => setParams. * Support anonymous API usage of searching and listing for skills or collections. allow for rate limiting public requests with bucket4j * Rebase PR and update new test data - also reverted change to MockData for how RSDs relate to connections Co-authored-by: wgu-edwin <[email protected]> Co-authored-by: John Kallies <[email protected]> Co-authored-by: Roberto Meza <[email protected]> Co-authored-by: Devon Sumner <[email protected]> Co-authored-by: drey-bigney <[email protected]>
JohnKallies
added a commit
that referenced
this pull request
Oct 25, 2021
* 1.0.5-SNAPSHOT * DMND-414: Remove DELETE methods from ngp-aa-osmt-osn * DMND-519 Break up ElasticSearch reindexing into smaller transactions * Renaming class to be more clear * removed test file and secrets file added nginx folder with two nginx files * created node script file that finds kube files and replaces template info with customer info * templating the kube files created a template placeholder on all necessary places to deploy by client * Fixed the grammar of the search result. Used to say: 2421 RSDs found based on and your filters. Viewing 1-50. The 'and' has been moved inside the conditional, so it will only appear when there is something to 'and' with. * Fixed bug: '1 collection' was sometimes '1 collections' (found through new test) * Fixed bug: '1 RSD' was sometimes '1 RSDs' (found through new test) * Removed osn pilot kube files * added a new file called license (#4) Added Apache 2.0 License * added code of conduct file (#6) * Contribution (#7) * Created first draft of contribution file * added link to issues and removed unecessary links * Feature/multiple alignments (#8) * ApiSkill.alignments is now has its own type: ApiAlignment instead of an ApiNamedReference * OSMT-1217: support multiple alignments on create/edit skill form * OSMT-1224: add 'framework' field to Keyword data model and 'isPartOf' field to ApiAlignment * OSMT-1217: include alignment framework on create/edit skill form * OSMT-1217: dynamic alignment forms should contribute to overall form valid/pristine state * OSMT-1219: display multiple alignments on Rich Skill Manage and published skill pages * OSMT-1222: include all alignments in RichSkillCsvExport * OSMT-1221: support multiple alignments in batch import * OSMT-1221: batch import - unambiguous mapping for multiple alignments * required form validation for Alignment name/URL OSMT-1248 * update batch import template to include multiple alignments and framework * sort alignments alphabetically by framework/skillName on manage/public lists OSMT-1258 * update batch import property definitions copy for alignment name and framework OSMT-1221 * Update copy for alignment name field * add missing 's' to Alignments field label on published skill page * final copy for alignment form fields OSMT-1260 * backwards incompatible API changes necessitate a major version bump in the API spec. * Create maven.yml * Final repo check (#11) Removed references to internals * Updated build action * Fixed Junit test case failure. (#13) * Fixed Junit test case failure. * Removed argument to skip test cases during build process from dockerfile & Readme.md * added a clarifying statement under attribution (#17) as per legal department, I have added the statement under attribution about the contribution covenant. * Update CONTRIBUTING.md (#22) Updated contribution guidelines * DMND-631 - Update code to include BLS JobCodes when the skills are im… (#26) * DMND-631 - Update code to include BLS JobCodes when the skills are imported using the UI batch import process. * DMND-631 - Addressed PR comments. * DMND-673 Update open source repo with osmt tests (#29) * DMND-674 Update open source repo with osmt-ui tests (#31) * Test improvements (#32) * * Added more tests * Upgraded testing dependencies to support code coverage. * Missed 2 files. * * Removed isAuthorEditable() in many cases because the intent was separate from using the defaultAuthorValue, but was not coded separately. i.e., if there is a defaultAuthorValue, then use it as the default. (#33) * Fixed the assumption that the same web server is used for both UI and Service. That's not always the case. * Temporary handling of the ElasticSearch limitation of 10000 RSDs. Until that is fixed by other means, we add a "+" to reduce confusion about the 10000 RSD count. * Add noauth profile config for local development (#34) * Add noauth profile config for local development SecurityConfigNoAuth.kt taken from ngp-aa-osmt * Add noauth profile config for local development * Update readme (#9) * added architectural diagram and BLS Onet import process * added Okta steps for setup * inserted PNG to assests folder to use in README file for the architectural diagram * fixed png so that it removed the box with the osmt description. * Closes #18 - Update README for community contributors (#39) * Closes #18 - Update README for community contributors also updated the API module's README * Address review feedback for project README and docs - also made a small tweak to .gitignore to support multiple env files. * Readme formatting * Add Testing Expections to CONTRIBUTING.md (#43) * Add Testing Expections to CONTRIBUTING.md - clarified the OSMT definition of unit test / integration test / e2e test * Update CONTRIBUTING.md Co-authored-by: drey-bigney <[email protected]> * Adding test for ElasticSearchReindexer (#45) * Fix issues with docker-compose (#41) * Fix issues with docker-compose - required changes in Dockerfile around yum install for certain openjdk version - required skipping tests when building app Docker image because unit tests are mixed with integration tests that rely on Dockerized services - fixed missinf fat jar functionality with UI as a dependency for API * Update pom.xml * Aadjust Dockerfile for review requests - excluded defacto integration tests from Dockerfile mvn package with "dockerfile-build" Maven profile * add new integration test to pom file for exclusions in docker build * Feature/add code coverage (#46) * * Added more tests * Upgraded testing dependencies to support code coverage. * Missed 2 files. * * Added more tests. * Added test for RichSkillSearchResultsComponent. Also added support for ActivatedRouteStub.setQueryParams. * Renamed ActivatedRouteStub.setParamsMap => setParams. * Allow anonymous API acess to search and list endpoints for published skills and collections (#16) * Support anonymous API usage of searching and listing for skills or collections. allow for rate limiting public requests with bucket4j * added a clarifying statement under attribution (#17) as per legal department, I have added the statement under attribution about the contribution covenant. * Update CONTRIBUTING.md (#22) Updated contribution guidelines * DMND-631 - Update code to include BLS JobCodes when the skills are im… (#26) * DMND-631 - Update code to include BLS JobCodes when the skills are imported using the UI batch import process. * DMND-631 - Addressed PR comments. * DMND-673 Update open source repo with osmt tests (#29) * DMND-674 Update open source repo with osmt-ui tests (#31) * Test improvements (#32) * * Added more tests * Upgraded testing dependencies to support code coverage. * Missed 2 files. * * Removed isAuthorEditable() in many cases because the intent was separate from using the defaultAuthorValue, but was not coded separately. i.e., if there is a defaultAuthorValue, then use it as the default. (#33) * Fixed the assumption that the same web server is used for both UI and Service. That's not always the case. * Temporary handling of the ElasticSearch limitation of 10000 RSDs. Until that is fixed by other means, we add a "+" to reduce confusion about the 10000 RSD count. * Add noauth profile config for local development (#34) * Add noauth profile config for local development SecurityConfigNoAuth.kt taken from ngp-aa-osmt * Add noauth profile config for local development * Update readme (#9) * added architectural diagram and BLS Onet import process * added Okta steps for setup * inserted PNG to assests folder to use in README file for the architectural diagram * fixed png so that it removed the box with the osmt description. * Closes #18 - Update README for community contributors (#39) * Closes #18 - Update README for community contributors also updated the API module's README * Address review feedback for project README and docs - also made a small tweak to .gitignore to support multiple env files. * Readme formatting * Add Testing Expections to CONTRIBUTING.md (#43) * Add Testing Expections to CONTRIBUTING.md - clarified the OSMT definition of unit test / integration test / e2e test * Update CONTRIBUTING.md Co-authored-by: drey-bigney <[email protected]> * Adding test for ElasticSearchReindexer (#45) * Fix issues with docker-compose (#41) * Fix issues with docker-compose - required changes in Dockerfile around yum install for certain openjdk version - required skipping tests when building app Docker image because unit tests are mixed with integration tests that rely on Dockerized services - fixed missinf fat jar functionality with UI as a dependency for API * Update pom.xml * Aadjust Dockerfile for review requests - excluded defacto integration tests from Dockerfile mvn package with "dockerfile-build" Maven profile * add new integration test to pom file for exclusions in docker build * Feature/add code coverage (#46) * * Added more tests * Upgraded testing dependencies to support code coverage. * Missed 2 files. * * Added more tests. * Added test for RichSkillSearchResultsComponent. Also added support for ActivatedRouteStub.setQueryParams. * Renamed ActivatedRouteStub.setParamsMap => setParams. * Support anonymous API usage of searching and listing for skills or collections. allow for rate limiting public requests with bucket4j * Rebase PR and update new test data - also reverted change to MockData for how RSDs relate to connections Co-authored-by: wgu-edwin <[email protected]> Co-authored-by: John Kallies <[email protected]> Co-authored-by: Roberto Meza <[email protected]> Co-authored-by: Devon Sumner <[email protected]> Co-authored-by: drey-bigney <[email protected]> * Revert "Allow anonymous API acess to search and list endpoints for published skills and collections (#16)" (#53) This reverts commit 4e34f7c. * update Maven dependency (#54) - update from 3.8.1 to 3.8.3 - 3.8.1 was no longer avaialble at the previous location. Aadded a more general URL * update config changes for dev docker-compose deployment (#51) * update config changes for dev docker-compose deployment * add documentation covering dev stack and general config * Updated readme for development stack configurations. - Added details to set env variables Added sample import files. Co-authored-by: karan.singh <[email protected]> * update README for Quickstart configs (#56) * update config changes for docker-compose deployments * update quickstart config changes for docker-compose deployments * update README for Quickstart configs * housekeeping for previous change * Fixed to show this statement in italic. * remove REINDEX_ELASTICSEARCH * remove unused import in PropertyLogger * remove orphaned code in docker_entrypoint script * fix typo in docker volume path * fix erroenous console message in docker_entrypoint script * add FRONTEND_URL as env var in docker-compose.yml * re-add REINDEX_ELASTICSEARCH in docker_entrypoint Co-authored-by: karan.singh <[email protected]> * Changes for import functionality. (#60) * Changes for import functionality. * Deleted the cmd to import skills. Co-authored-by: Wiggins <[email protected]> Co-authored-by: Cam Peterson <[email protected]> Co-authored-by: campeterson22 <[email protected]> Co-authored-by: Stirling Hale <[email protected]> Co-authored-by: stirhale-wgu <[email protected]> Co-authored-by: Edwin Santizo <[email protected]> Co-authored-by: Devon Sumner <[email protected]> Co-authored-by: Devon Sumner <[email protected]> Co-authored-by: drey-bigney <[email protected]> Co-authored-by: Drey Bigney <[email protected]> Co-authored-by: wgu-edwin <[email protected]> Co-authored-by: karan-beyond <[email protected]> Co-authored-by: Roberto Meza <[email protected]> Co-authored-by: karan.singh <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
are mixed with integration tests that rely on Dockerized services