ci 공사중.. #259
ci 공사중.. #259
Conversation
WalkthroughThe changes introduce a new GitHub Actions workflow for CI using Gradle, automate the build and test reporting process, and update key generation in the Apple OAuth client by replacing file-based key retrieval with direct property access and specific exception handling. The Apple OAuth client properties record now includes a new secretKey field. Multiple test classes have had the Changes
Sequence Diagram(s)sequenceDiagram
participant PR as "GitHub PR"
participant CI as "CI Workflow"
participant Code as "Repository Code"
participant Build as "Gradle Build"
PR->>CI: PR event on develop/release/master
CI->>Code: Checkout repository
CI->>CI: Set up JDK 17 (Temurin)
CI->>CI: Install Gradle
CI->>Build: Execute './gradlew build'
Build-->>CI: Return build & test reports
CI->>PR: Report outcome
sequenceDiagram
participant Context as "Spring Application Context"
participant RTC as "RedisTestContainer"
participant Env as "Application Environment"
Context->>RTC: Call initialize()
RTC->>RTC: Start Redis container on port 6379
RTC->>Context: Set Redis properties via TestPropertyValues
Context-->>RTC: Continue with test initialization
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This pull request updates test configurations and container setups while introducing modifications to client secret generation for Apple OAuth. Key changes include:
- Adding a new application.yml configuration file for test resources.
- Removing the usage of @activeprofiles("test") in favor of more explicit container initializers and context configurations.
- Refactoring AppleOAuthClientSecretProvider to generate the private key using a secret key from properties rather than reading from a file.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/resources/application.yml | Adds configuration for database, cloud, OAuth, and other test-specific settings. |
| src/test/java/com/example/solidconnection/support/TestContainerSpringBootTest.java | Replaces @activeprofiles with @ContextConfiguration and refactors test container imports. |
| src/test/java/com/example/solidconnection/support/TestContainerDataJpaTest.java | Removes the ActiveProfiles test annotation to streamline test context setup. |
| src/test/java/com/example/solidconnection/support/RedisTestContainer.java | Modifies the container startup using a static block and implements ApplicationContextInitializer. |
| src/test/java/com/example/solidconnection/support/DatabaseCleaner.java, RedisConnectionTest.java, DatabaseConnectionTest.java | Removes ActiveProfiles annotations. |
| src/test/java/com/example/solidconnection/community/comment/service/CommentServiceTest.java | Adjusts timestamp assertions using withNano(0) to compare datetimes consistently. |
| src/main/java/com/example/solidconnection/config/client/AppleOAuthClientProperties.java | Introduces a new property for secretKey to support OAuth client secret generation. |
| src/main/java/com/example/solidconnection/auth/client/AppleOAuthClientSecretProvider.java | Refactors private key generation to use the secret key from properties and adapts exception handling. |
| .github/workflows/ci.yml | Adds a CI workflow for building the project with Gradle on pull requests. |
Comments suppressed due to low confidence (1)
src/main/java/com/example/solidconnection/auth/client/AppleOAuthClientSecretProvider.java:63
- The error code 'FAILED_TO_READ_APPLE_PRIVATE_KEY' is misleading as the private key is now generated from properties rather than read from a file. Consider updating this error code and its corresponding message to reflect the new implementation.
throw new CustomException(FAILED_TO_READ_APPLE_PRIVATE_KEY);
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here. PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
src/main/java/com/example/solidconnection/config/client/AppleOAuthClientProperties.java (1)
13-14: Add comma afterkeyIdfield and add Javadoc for the new propertyThe addition of the
secretKeyfield is correctly implemented syntactically. However, you should consider adding documentation to explain the purpose and expected format of this new field, especially since it appears to store sensitive information based on the application.yml configuration.String clientId, String teamId, - String keyId, - String secretKey + String keyId, + /** The private key for Apple OAuth in base64 encoded format */ + String secretKey.github/workflows/ci.yml (2)
1-31: Good implementation of CI workflow with room for improvementThe CI workflow looks good overall. It correctly sets up JDK 17, Gradle, and runs the build process. However, there are a few improvements that could be made:
- Add caching for Gradle dependencies to speed up subsequent builds:
- name: Setup Gradle uses: gradle/actions/setup-gradle@v4 + with: + cache-read-only: ${{ github.ref != 'refs/heads/master' && github.ref != 'refs/heads/develop' && github.ref != 'refs/heads/release' }}
- Add test result reporting for better visibility of test failures:
- name: Build with Gradle Wrapper run: ./gradlew build + + - name: Publish Test Report + uses: mikepenz/action-junit-report@v3 + if: success() || failure() + with: + report_paths: '**/build/test-results/test/TEST-*.xml'
3-5: Consider adding workflow_dispatch trigger for manual runsCurrently, the workflow only runs on pull requests. Adding a workflow_dispatch trigger would allow for manual execution when needed.
on: pull_request: branches: [ "develop", "release", "master" ] + workflow_dispatch:src/test/resources/application.yml (5)
9-17: Reconsider using "create" for ddl-auto in test environmentUsing
ddl-auto: createwill recreate the schema every time the application starts. For tests, consider usingcreate-dropinstead to ensure a clean state between test runs.jpa: hibernate: - ddl-auto: create + ddl-auto: create-drop generate-ddl: true show-sql: true
67-70: Improve security for JWT secretThe JWT secret should be more complex and ideally loaded from an environment variable, even in test environments.
jwt: secret: - 1234567-1234-1234-1234-12345678901 + ${JWT_SECRET:a-more-complex-default-secret-for-testing-purposes-only-do-not-use-in-production}
23-26: Use placeholder values for AWS credentialsEven though these are test credentials, it's better to use clearly labeled placeholder values to prevent accidental usage of these values in other environments.
aws: credentials: - access-key: access-key - secret-key: access-key + access-key: test-access-key + secret-key: test-secret-key
47-59: Structure OAuth configuration consistentlyThe Kakao OAuth configuration is not properly indented, making it inconsistent with the Apple OAuth configuration.
oauth: apple: token-url: "https://appleid.apple.com/auth/token" client-secret-audience-url: "https://appleid.apple.com" public-key-url: "https://appleid.apple.com/auth/keys" client-id: client-id team-id: team-id key-id: key-id redirect-url: "https://localhost:8080/auth/apple" secret-key: MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCAfGIQ3TtNYAZG7i3m72odmdhfymkM9wAFg2rEL2RKUEA== -kakao: - redirect-url: "http://localhost:8080/auth/kakao" - client-id: client-id - token-url: "https://kauth.kakao.com/oauth/token" - user-info_url: "https://kapi.kakao.com/v2/user/me" + kakao: + redirect-url: "http://localhost:8080/auth/kakao" + client-id: client-id + token-url: "https://kauth.kakao.com/oauth/token" + user-info_url: "https://kapi.kakao.com/v2/user/me"🧰 Tools
🪛 Gitleaks (8.21.2)
54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
55-59: Fix typo in Kakao OAuth user info URL property nameThere's a typo in the property name:
user-info_urlshould be eitheruser-info-url(kebab-case) oruser_info_url(snake_case), but not a mix of both.kakao: redirect-url: "http://localhost:8080/auth/kakao" client-id: client-id token-url: "https://kauth.kakao.com/oauth/token" - user-info_url: "https://kapi.kakao.com/v2/user/me" + user-info-url: "https://kapi.kakao.com/v2/user/me"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/ci.yml(1 hunks)src/main/java/com/example/solidconnection/auth/client/AppleOAuthClientSecretProvider.java(3 hunks)src/main/java/com/example/solidconnection/config/client/AppleOAuthClientProperties.java(1 hunks)src/test/java/com/example/solidconnection/community/comment/service/CommentServiceTest.java(2 hunks)src/test/java/com/example/solidconnection/database/DatabaseConnectionTest.java(0 hunks)src/test/java/com/example/solidconnection/database/RedisConnectionTest.java(0 hunks)src/test/java/com/example/solidconnection/support/DatabaseCleaner.java(0 hunks)src/test/java/com/example/solidconnection/support/RedisTestContainer.java(1 hunks)src/test/java/com/example/solidconnection/support/TestContainerDataJpaTest.java(0 hunks)src/test/java/com/example/solidconnection/support/TestContainerSpringBootTest.java(2 hunks)src/test/resources/application.yml(1 hunks)
💤 Files with no reviewable changes (4)
- src/test/java/com/example/solidconnection/database/RedisConnectionTest.java
- src/test/java/com/example/solidconnection/support/DatabaseCleaner.java
- src/test/java/com/example/solidconnection/support/TestContainerDataJpaTest.java
- src/test/java/com/example/solidconnection/database/DatabaseConnectionTest.java
🧰 Additional context used
🪛 Gitleaks (8.21.2)
src/test/resources/application.yml
54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (10)
src/test/java/com/example/solidconnection/support/TestContainerSpringBootTest.java (1)
16-20:Details
❓ Verification inconclusive
Verify Redis configuration after changing how the container is initialized
The test configuration has been modified to use RedisTestContainer as an initializer instead of importing it directly. This is a good approach for container initialization, but ensure that all tests dependent on this annotation still work correctly with the new configuration.
🏁 Script executed:
#!/bin/bash # Check for Redis-dependent tests that might be affected by this change echo "Searching for tests using Redis..." rg -l "RedisTestContainer|TestContainerSpringBootTest" --type java src/testLength of output: 2321
Action Required: Validate Redis Container Initialization in All Affected Tests
The test configuration now uses
RedisTestContaineras a context initializer instead of importing it directly, which is a good approach for container initialization. However, the grep results show that several tests referenceRedisTestContainer(e.g., inGeneralUniversityRecommendServiceTest.java,RedisTestContainer.java, and others). Please verify that every test dependent on Redis is correctly initialized and functions as expected with this change.
- Confirm that tests under
src/test/java/com/example/solidconnection/support/and other Redis-related tests (listed in the grep output) execute properly.- Double-check integration flows that rely on Redis to ensure no unintended side effects occurred from the altered initialization strategy.
src/test/java/com/example/solidconnection/community/comment/service/CommentServiceTest.java (2)
74-77: Improved timestamp comparison for robust testingThe modification to ignore nanoseconds in timestamp comparisons is a good practice. This prevents test failures due to minor precision differences that aren't functionally relevant.
94-97: Consistent timestamp assertion pattern applied to child commentsGood consistency in applying the same nanosecond-ignoring pattern to child comment timestamp assertions, ensuring all timestamp comparisons are equally robust.
src/main/java/com/example/solidconnection/auth/client/AppleOAuthClientSecretProvider.java (4)
13-15: Improved exception handling with specific exception importsAdding specific exception imports enhances code clarity for the exception handling in the private key generation method.
22-22: Grammar correction in documentation commentMinor grammatical improvement in the Korean documentation comment.
38-38: Method name change reflects new implementation approachChanged from
readPrivateKey()togeneratePrivateKey()to better reflect the actual implementation that no longer reads from a file.
56-66: Refactored key generation to use property-based configurationThis is a significant improvement that:
- Eliminates file I/O operations which could fail due to file access issues
- Makes configuration more flexible through property-based approach
- Improves security by not requiring key files on disk
- Provides more precise exception handling
The implementation is clean and properly handles potential exceptions.
src/test/java/com/example/solidconnection/support/RedisTestContainer.java (3)
3-8: Improved test container initialization through ApplicationContextInitializerThis change adopts a more programmatic approach to test container initialization by implementing the ApplicationContextInitializer interface, giving more control over the container lifecycle.
10-16: Simplified container configuration with explicit port mappingThe new approach:
- Defines a constant for the original Redis port
- Configures the container with explicit port exposure
- Uses a static initialization block for immediate container startup
This provides clearer and more deterministic container configuration.
18-24: Programmatic property injection for Redis test configurationThe implementation of the initialize method uses TestPropertyValues to inject Redis host and port settings directly into the application context, making the test configuration more explicit and closely tied to the container lifecycle.
| team-id: team-id | ||
| key-id: key-id | ||
| redirect-url: "https://localhost:8080/auth/apple" | ||
| secret-key: MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCAfGIQ3TtNYAZG7i3m72odmdhfymkM9wAFg2rEL2RKUEA== |
There was a problem hiding this comment.
Security risk: Private key exposed in the configuration file
The secret-key property for Apple OAuth contains what appears to be a base64-encoded private key directly in the configuration file. Even for a test environment, it's a security risk to include private keys in source code.
Consider using environment variables or a secure vault solution even for test environments:
- secret-key: MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCAfGIQ3TtNYAZG7i3m72odmdhfymkM9wAFg2rEL2RKUEA==
+ secret-key: ${APPLE_SECRET_KEY:test-key}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| secret-key: MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCAfGIQ3TtNYAZG7i3m72odmdhfymkM9wAFg2rEL2RKUEA== | |
| secret-key: ${APPLE_SECRET_KEY:test-key} |
🧰 Tools
🪛 Gitleaks (8.21.2)
54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
- create-drop을 create로 변경 - 테스트 컨테이너를 실행해서 테스트하기 때문에, 테스트가 종료되면 당연히 테이블이 삭제된다. 따라서 create-drop을 하는것과 create를 하는 것이 동일하게 동작한다. 굳이 create로 바꾼 것은 삭제 시 FK 위반으로 WARN 로그가 불필요하게 백몇줄이 찍히는걸 막기 위해서.
- 깨지는 테스트 수정 - 비지니스 로직 변경으로 더 이상 해당되지 않는 테스트 삭제
nayonsoso
force-pushed
the
chore/111-ci
branch
2 times, most recently
from
3766eee to
0960ce0
Compare
1 participant
User description
관련 이슈
작업 내용
특이 사항
리뷰 요구사항 (선택)
PR Type
Enhancement, Tests, Configuration changes
Description
Refactored
AppleOAuthClientSecretProviderto usesecretKeyfrom properties.Added
secretKeyfield toAppleOAuthClientProperties.Updated test configurations to remove
ActiveProfilesand improve Redis TestContainer setup.Added CI workflow for Gradle builds and a dedicated
application.ymlfor testing.Changes walkthrough 📝
2 files
Refactored to use `secretKey` from propertiesAdded `secretKey` field for Apple OAuth7 files
Updated time comparison to ignore nanosecondsRemoved `ActiveProfiles` annotationRemoved `ActiveProfiles` annotationRemoved `ActiveProfiles` annotationImproved Redis TestContainer initializationRemoved `ActiveProfiles` annotationUpdated Redis TestContainer initialization2 files
Added CI workflow for Gradle buildsAdded test-specific application configurationSummary by CodeRabbit