-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: NCP 인프라를 테라폼을 활용하여 구성합니다 #9
Conversation
- name: Terraform Apply | ||
working-directory: ${{ env.working-directory }} | ||
if: github.ref == 'refs/heads/main' && github.event_name == 'push' | ||
run: terraform apply -auto-approve -var=ncp_access_key=${{ secrets.NCP_ACCESS_KEY_ID }} -var=ncp_secret_key=${{ secrets.NCP_SECRET_ACCESS_KEY }} -input=false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 GitHub Actions 워크플로우 코드는 Terraform을 사용하여 인프라를 관리하는 것처럼 보입니다. 몇 가지 간단한 건의를 내릴 수 있습니다:
-
기밀정보: 'TF_API_TOKEN', 'NCP_ACCESS_KEY_ID', 'NCP_SECRET_ACCESS_KEY' 같은 비밀 정보들이 사용되고 있는데, 이런 중요한 정보는 꼭 저장소나 코드에 직접적으로 드러나지 않도록 환경 변수나 secrets를 통해서 처리하셔야 합니다. 현재 이 부분은 잘 설정된 것으로 보입니다.
-
브랜치 이름: 'infra/main' 이라는 이름이 참조되었지만, 실제로 Terraform Apply 단계는 'refs/heads/main' 브랜치에서만 실행됩니다. 이는 언급된 브랜치 이름과 일치하지 않기 때문에, 스크립트의 의도에 따라 올바른 브랜치명을 사용하도록 고려해보시면 좋겠습니다.
-
에러 처리: Terraform 작업 중 발생할 수 있는 잠재적인 오류를 잡아내고 워크플로우를 실패로 마크하는 보다 견고한 에러 처리 메커니즘이 필요합니다.
-
Terraform 버전: 프로젝트에서 지원하는 Terraform의 버전(현재 1.8.2로 설정)을 항상 최신으로 유지하는 것이 좋습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4832ac5 커밋에서 main -> infra/main
으로 수정하였습니다!
secret_key = var.ncp_secret_key | ||
rds_username = var.ncp_rds_username | ||
rds_password = var.ncp_rds_password | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기에 제가 찾을 수 있는 몇 가지 potential bug risk 및 개선 사항이 있습니다:
-
이 코드는 Terraform cloud 및 NCP(Naver Cloud Platform) provider 모듈의 정의를 포함하고 있습니다. 하지만 사용한 변수들(var.prefix, var.ncp_region 등)의 정의를 찾을 수 없습니다. 이 값을 어디에서 설정하고 있는지 확인해야 합니다.
-
또한, ncp_access_key, ncp_secret_key, ncp_rds_username, ncp_rds_password와 같은 보안 관련 정보가 노출될 위험이 있습니다. 이 정보는 이 코드가 public으로 공유되면 심각한 보안 문제를 초래할 수 있습니다. 환경 변수나 Vault를 사용하여 이러한 중요 키를 보호하는 것을 고려해 보세요.
-
Terraform workspace 이름('few-org-work')이 하드 코딩되어 있습니다. 다른 사용자가 이 코드를 가져가서 실행한다면 문제가 발생할 수 있습니다. 워크스페이스 이름 역시 변수로 만들어준다면 사용자가 쉽게 수정해서 사용할 수 있을 것입니다.
-
제공된 terraform 코드 주석이 부족합니다. Terraform 코드에 대한 충분한 주석은 유지 보수성을 향상시킵니다.
-
마지막으로, 버전 제어가 없는 듯 보입니다. 사용하는 테라폼과 NCP provider의 버전을 직접 명시하면, 이후 버전 업데이트에 따른 영향을 미리 방지할 수 있습니다.
따라서, 상기 사항들을 고려하여 코드를 수정하거나 개선해보는 것이 좋겠습니다.
port_range = "1-65535" | ||
description = "accept 1-65535 port" | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 리뷰는 우선적으로 다음 세가지 문제점을 지적하려 합니다:
-
보안 취약성: "0.0.0.0/0" 설정은 모든 IP 주소에서의 접근을 허용하는 것이므로 해커들에게 넓은 사후 범위를 제공하는 보안 위험성이 있습니다. 만약 꼭 필요한 경우가 아니라면 이 설정을 재고해 보세요.
-
포트 관리: "1-65535" 포트를 열어 놓는 것은 실제 필요한 서비스를 위한 포트 외에도 넓은 범위의 포트를 개방하고 있다는 이슈가 있습니다. 필요한 포트만 개방하는 것이 좋으며, 가급적 넓은 범위의 포트 개방을 회피해야 합니다.
-
중복된 설정: 5번째 부분인 'inbound' 설정에서 (ip_block = "0.0.0.0/0", port_range = "3306") 동일한 포트와 IP 대역에 대해 중복하여 설정하였습니다. 이런 중복되는 설정이 있으면 큰 이슈는 아니지만, 코드의 효율성과 관리 측면에서 바람직하지 않으니 수정 고려해 보세요.
개선 방향으로는 필요한 호스트와 포트에 대한 충분한 조사 후, 필요에 따라 접근 제어 그룹 규칙을 세분화하고 보안 조치를 강화하는 것이 좋겠습니다.
is_backup = false | ||
port = 3306 | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치는 Terraform으로 작성되어, nCloud에서 MySQL 데이터베이스를 생성하는 역할을 합니다. 여러가지 측면을 고려해보자면 다음과 같습니다:
- 보안:
user_name
과user_password
가 직접 코드에 포함되지 않도록 템플릿 변수를 사용하고 있습니다. 이 부분은 잘 해결된 것입니다. - 데이터 무결성 및 가용성:
is_ha
가 false로 설정돼 있습니다. 이는 고가용성(HA) 설정을 활성화하지 않습니다. 중요한 서비스를 위한 데이터베이스인 경우 HA 설정을 true로 하는 것을 고려해야 합니다. 같은 맥락에서,is_backup
이 false로 설정돼 있는데, 데이터 손실 위험을 방지하기 위해 백업 옵션을 사용하는 것을 고려하세요. - 리소스 체계: 모든 자원 이름이
${var.prefix}-db
에 기반하여 설정되어 있습니다. 이것은 처음에 식별하기 쉽지만, 차후 관리나 수정 시 혼돈을 초래할 수 있습니다. 특정 목적이나 규칙에 따른 좀 더 명확한 네이밍 전략이 필요할 지도 모릅니다. - SSD 스토리지:
data_storage_type
이 "SSD"로 설정되어 있습니다만, 요구 사항에 따라서는 다른 스토리지 타입을 사용해야 할 수도 있습니다.
아직까지 체크 리스트와 관련된 모든 사항에 대한 필요성 및 그 영향력에 대해 팀과 함께 검토하실 것을 권장합니다.
protocol = "HTTP" | ||
port = 80 | ||
target_group_no = ncloud_lb_target_group.be_tg.target_group_no | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치는 Terraform을 사용하여 NCloud의 로드 밸런서와 리스너를 생성합니다. 구성에 대한 첫 번째 소견은 다음과 같습니다:
- 입력 값
var.prefix
가 존재하는지 확인하십시오. 선언되지 않은 변수를 사용하려고 하면 실행 시 오류가 발생합니다. ncloud_subnet.lb_a.subnet_no
나ncloud_lb_target_group.be_tg.target_group_no
이런 자원들이 선언되고, 생성되어 있는지 확인하세요. 이들 없이 해당 로드밸런서와 리스너는 생성되지 않을 것입니다.- 리스너 프로토콜이 "HTTP"로 설정되어 있습니다. HTTPS가 필요하다면 변경해야 합니다.
- 포트 80이 열려있는지 확인하세요. 그렇지 않으면 연결할 수 없습니다.
- 코드의 향상을 위해 입력변수와 출력변수를 정의하여 모듈화 할 수 있습니다. 이렇게 함으로써 재사용성과 유지 관리가 간편해집니다.
마지막으로, Terraform plan 나 Terraform apply 명령을 써서 변화를 보고 미리 확인할 수 있는 건 좋은 습관입니다.
이 외에도 작업환경, 사용 중인 NCloud 서비스 등의 상황에 따라 추가적인 검토 사항이 있을 수 있습니다. 이 문맥에서만 제시할 수 있는 조언입니다.
access_key = var.access_key | ||
secret_key = var.secret_key | ||
region = var.region | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 Terraform 코드 패치는 전반적으로 잘 작성된 것 같습니다. 가장 기본적인 보안 요소(액세스 키 및 비밀 키)가 변수로 처리되어 직접 코드에 포함되지 않아 좋습니다.
그러나 개선할 여지가 조금 있으니, 아래 두 가지를 고려해 보세요:
-
특정 값을 하드코딩하지 않고 하는 것에는 큰 이점이 있지만, 최소의 Terraform 버전(
0.13
)은 하드 코딩되어 있습니다. 이 값을 변수로 만들거나 설정 파일에서 가져올 수 있도록 고려해보세요. 그렇게 할 경우 Terraform의 버전을 좀 더 유동적으로 관리할 수 있습니다. -
VPC 지원 사용 여부(
support_vpc
값)도 코드에 하드 코딩되어 있습니다. 필요에 따라 이 값을 변경하려면 코드를 직접 수정해야 합니다. 이 값을 외부에서 설정할 수 있게 변수로 선언하는 것이 좋을 것 같습니다.
마지막으로, 코드가 실제 환경에서 어떻게 동작하는지 확인하기 위해 테스트를 꼭 해 보세요!
description = "Backend NIC" | ||
subnet_no = ncloud_subnet.public_a.subnet_no | ||
access_control_groups = [ncloud_access_control_group.be_server.id] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치를 검토하는 것은 어렵지 않습니다. 주로 Terraform을 사용하여 ncloud에 네트워크 인터페이스를 생성하는 것으로 보입니다.
-
오류 발견: 확신할 수 없지만, 모든 입력 변수(
${var.prefix}
등)와 모든 리소스 참조(ncloud_subnet.public_a.subnet_no
및ncloud_access_control_group.be_server.id
)가 올바르게 정의되어 있는지 확인해야 합니다. -
개선 제안: 변수를 직접 문자열에 합성하기보다는, Terraform이 권장하는 방식인 string interpolation syntax 를 사용하고 있습니다. Terraform 0.12 이상에서는 다음과 같이 단순한 문자열 연결을 선호합니다:
name = var.prefix + "-be-server-nic"
- 가독성: 기능에는 전혀 문제가 없지만, 코드 가독성을 위해 각 자원에 대한 짧은 설명을 추가하는 주석을 고려할 수 있습니다.
제보된 코드 조각은 매우 간단하므로 버그 Risk는 많지 않습니다. 하지만 프로젝트의 다른 부분에서 이 변수와 자원을 적절하게 정의하고 관리하는지 확인하는 것이 중요합니다.
resource "ncloud_route_table_association" "public_a" { | ||
route_table_no = ncloud_route_table.public_rt.id | ||
subnet_no = ncloud_subnet.public_a.id | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 리뷰를 진행하였습니다. 발생할 수 있는 오류 위험과 제안 사항은 아래와 같습니다:
-
변수 확인:
var.prefix
,ncloud_vpc.vpc.id
및ncloud_subnet.public_a.id
와 같은 변수 값이 적절한지 항상 확인하고, 유효한지 보장하십시오. 더 안정적인 코드를 위해 오류 처리 메커니즘이 있는지 확인하는 것이 좋습니다. -
리소스 이름 변경을 고려하세요: 리소스의 name 필드 (
"${var.prefix}-public-rt"
와"${var.prefix}-private-rt"
)가 중복될 가능성이 있으면 충돌이 발생할 수 있습니다. 이름에 일부 고유한 ID를 포함하거나 타임스탬프를 사용하여 잠재적 충돌을 방지하는 것이 좋습니다. -
개인 네트워크를 위한 Route Table Association 작성이 누락되었습니다: 본 코드는 공용 서브네트에 대한 Route Table Association만 생성합니다. 개인(subnet type이 'PRIVATE'인) 서브네트에 대한 관계도 설정할 계획이라면 해당 코드를 추가해야 할 것입니다.
-
테라폼 버전 및 제공자 버전: 코드에서 명시적으로 테라폼과 ncloud 제공자의 버전을 지정하지 않았습니다. 기능이 제대로 작동하는지 확인하려면 필요한 최소 버전을 지정해야 합니다.
-
에러 처리: Terraform은 Fail-fast 모델을 따르므로, 첫 번째 오류가 발생하면 실행이 중단됩니다. 이 경우, 다루기 쉬운 오류 메시지를 출력해주는 무언가가 있으면 좋겠습니다.
마지막으로, 이 코드 리뷰는 테라폼 HCL (HashiCorp Configuration Language)의 활용에 초점을 맞추고 있으며, 기능적인 부분은 고려하지 않았음을 참고해주시기 바랍니다.
systemctl start docker | ||
docker run -d -p 8080:80 --name nginx nginx | ||
EOF | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드는 Terraform을 사용하여 Naver Cloud Platform(NCP)에 서버를 생성하며, 해당 서버에 Docker 및 Nginx를 설정하는 예입니다. 제가 파악한 여러 가지 개선해야 할 부분은 아래와 같습니다:
-
key 파일의 보안: 정의된 SSH Key 파일(
"${var.prefix}_be.pem"
)과 root 비밀번호 파일("${var.prefix}_be_root_password.txt"
)은 현재 파일 시스템에 평문으로 저장됩니다. 이는 보안 위험이 있으므로, 이 파일들은 암호화되어있고 안전한 방식으로 저장되어야 합니다. -
명시적인 서버 이미지 선택: 현재는 특정 Ubuntu 20.04 이미지(
"SW.VSVR.OS.LNX64.UBNTU.SVR2004.B050"
)를 명시적으로 사용하고 있습니다. 이 방식은 추후 OS 버전 업데이트가 발생할 경우 유연성을 잃을 수 있습니다. 가능한 다른 방식으로 최신 버전의 OS 이미지를 동적으로 선택하는 방법을 고려할 수 있습니다. -
코드 중복: 서버 속성값인 'filter' 부분에 중복이 많아 좀 더 modular하고 DRY(Don't Repeat Yourself) 원칙을 따르게 코드를 개선해볼 수 있습니다.
-
Docker 설치와 구동: 초기 스크립트는 Docker를 설치한 후 바로 실행합니다. 이는 간단한 테스트에는 적합하지만, 실제 환경에서는 Docker를 사용하기 전에 보안 및 구성 설정이 추가적으로 필요합니다.
-
Nginx 설정: 'docker run -d -p 8080:80 --name nginx nginx'는 기본 Nginx 이미지를 가져와 실행합니다. 실제 상황에서는 보통 별도의 Nginx 설정이 요구되므로 더 세부적인 구성이 필요할 수 있습니다.
-
코드 형식화: Terraform 코드는 통일된 코드 스타일 가이드를 따르는 것이 좋습니다. Terraform의 'fmt' 명령을 사용하여 코드를 수집할 수 있습니다.
-
에러 핸들링: 초기 스크립트에서 명령어가 실패하면 스크립트가 진행됩니다. 각 단계마다 성공/실패를 검사하고 필요한 경우 종료하는 로직이 포함되어야 합니다.
기타 사항이라면, 모든 리소스가 올바른 'depends_on' 지시문을 가지고 있는지 확인하면 좋습니다. 이를 통해 생성 순서를 제어하고 의존성 문제를 방지할 수 있습니다.
network_acl_no = ncloud_vpc.vpc.default_network_acl_no | ||
subnet_type = "PUBLIC" | ||
name = "${var.prefix}-db-a" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드는 ncloud의 VPC에 Load Balancer, Public 그리고 Database 서브넷을 생성하는 Terraform 스크립트로 보입니다. 대체적으로 깔끔하게 작성되어 있지만 몇 가지 개선점을 선택할 수 있습니다:
-
하드코딩: 서브넷 CIDR 범위와 리소스 이름 접두사 등의 값들이 하드코딩되어 있습니다. 이러한 값을 변수로 바꾸면 더 유연한 코드를 작성할 수 있으며 재사용이 쉬워집니다.
-
'KR-1' 표시: 지역('zone')이 'KR-1'로 설정되어 있는데, 이 역시 변수로 바꿔주면 코드를 재사용하기가 더 쉽습니다.
-
보안 그룹: 일반적으로 서브넷을 생성할 때는 해당 네트워크에 맞는 보안 그룹 또한 생성합니다. 이 스크립트에서는 보안 그룹 생성이 누락된 것 같습니다.
-
PUBLIC subnet_type: 모든 서브넷이 'PUBLIC' 으로 설정되어 있습니다. 생성하려는 네트워크 아키텍처에 따라 각 서브넷의 'subnet_type'을 적합한 값으로 변경해야 할 수도 있습니다.
코드 검토를 진행할 때는 위와 같은 변수화, 재사용성, 보안 등의 기준을 사용하여 코드를 검토합니다. 이러한 피드백이 도움이 되었길 바랍니다.
resource "ncloud_lb_target_group_attachment" "be_tg_attachment" { | ||
target_group_no = ncloud_lb_target_group.be_tg.target_group_no | ||
target_no_list = [ncloud_server.be_server.id] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드는 Terraform을 이용하여 Ncloud VPC의 로드 밸런서 설정을 하는 것으로 보입니다. 버그 위험이나 개선사항에 대해 몇 가지 제안드리겠습니다.
- 하드코딩된 값들: 값들 중 일부가 하드코딩되어 있음을 주목할 필요가 있습니다. 접근 방식이 'HTTP', 포트 번호
8080
,url_path = "/"
등은 모두 변수로 처리하면 더 유연한 코드가 될 수 있습니다. - URL path todo: 주석에서 확인할 수 있는 URL 경로는 나중에 수정하려는 계획인 것 같습니다. 그러나 실제 구현에서 이를 빠뜨릴 가능성이 있으므로 이 점에 확실히 관심을 기울여야 합니다.
- 에러 처리: 이 코드 조각에는 실제 자원 생성하는 과정에서 오류가 발생하면 이를 알아차리고 반응할 방법이 없습니다. 예외 처리를 추가하는 것이 좋습니다.
- Resources 참조:
ncloud_vpc.vpc.id
와ncloud_server.be_server.id
과 같은 리소스 참조는 이미 생성된 Terraform 자원에서 가져옵니다. 이 자원들이 적절히 선언되고 생성됐는지 확인해볼 필요가 있다. - 변수명: 변수명에 약어를 많이 사용했는데, 이해하는 데 어려울 수 있습니다. 가능한 한 명확하고 의미있는 이름을 사용하는 것이 좋습니다.
위의 사항들은 코드의 품질과 유지 관리성을 향상시키는 데 도움이 될 수 있습니다.
type = string | ||
default = "thisisrdspassword@1" | ||
description = "The password for the RDS instance" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치의 주요 문제점은 민감한 데이터(예: 'access_key', 'secret_key', 'rds_password')가 기본값으로 설정되어 있다는 점입니다. 이에 따라 코드가 공개적으로 액세스될 수 있으면 이러한 정보가 노출될 위험이 있습니다.
해결책은 다음과 같습니다:
- 비밀번호, 액세스 키, 비밀 키 등과 같은 중요한 정보를 보호하기 위해, 이들을 환경 변수로 관리하거나 별도의 안전한 방법을 통해 관리를 권장합니다.
- 위의 정보를 일반 코드나 버전 관리 시스템에 직접 삽입하지 마세요.
또 이 코드는 "region" 변수의 기본값이 'KR'로 설정되어 있는데, 제대로 확인해보니 그것이 실제 AWS 리전인지를 확인해야 합니다. 만약 실제 리전이 아니라면 이는 예상치못한 버그를 유발할 수 있습니다.
마지막으로 주석('description')으로 더 많은 상세한 정보를 제공하여 코드의 가독성을 향상시킬 수 있습니다. 각 변수가 어디에서 어떻게 사용되는지 명확하게 설명하면 다른 개발자들이 코드를 이해하고 유지보수하는데 도움이 될 것입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
d66aec7 에서 수정, 해당 값 삭제 후 재생성
resource "ncloud_vpc" "vpc" { | ||
name = "${var.prefix}-vpc" | ||
ipv4_cidr_block = "10.0.0.0/16" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 작은 Terraform 코드 조각을 확인해본 결과, 큰 문제는 없어 보입니다. 주요 기능인 VPC 생성과 관련된 부분은 정상처럼 보입니다. 그러나 여기서 주의해야 할 몇 가지 사항들이 있습니다:
-
에러 처리: 코드에서 발생할 수 있는 실수나 예외를 항상 고려해야 합니다. 이 테라폼 코드에는 오류가 발생하는 경우를 처리하기 위한 방안이 없습니다. 존재하지 않은 변수나 잘못된 CIDR 블록을 사용한 경우를 생각해 볼 수 있습니다.
-
CIDR 범위: IPv4 CIDR 블록 "10.0.0.0/16"은 현재 하드 코딩되어 있습니다. 이것이 의도적으로 선택된 것이라면 문제가 되지 않지만, 더욱 유연한 코드를 위해 이를 변수화 시키는 것을 추천합니다.
-
코드 중복: 코드에 "${var.prefix}-vpc" 등의 하드코딩된 부분이 있습니다. 이런 반복성이 높은 코드는 변수나 모듈 등으로 처리하여 중복을 줄이는 것이 좋습니다.
-
리소스 이름: 리소스 이름에 직접적인 접두어를 포함시킨 것은 좋은 실천법인데, 이름 충돌을 피하고 리소스간의 관계를 명확하게 표현하는데 도움이 됩니다. 하지만 일관성을 유지하는 것이 중요하므로, 이 패턴을 코드 전반에 걸쳐 사용하려면 규칙을 설정해야 합니다.
-
문서화: 마지막으로, 주석이나 문서를 사용하여 코드가 무엇을 하는지를 명확히 설명해주는 것이 좋습니다. "@@ -0,0 +1,5 @@"와 같은 부분은 각 줄에서 어떤 변화가 일어났는지를 설명하는데 도움이 될 수 있습니다.
type = string | ||
default = "thisisrdspassword@1" | ||
description = "The password for the RDS instance" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치는 Terraform을 사용하여 인프라를 설정하는 것으로 보입니다. 코드를 살펴보니, 몇 가지 개선 사항들이 떠오릅니다:
-
민감한 정보:
ncp_access_key
,ncp_secret_key
,ncp_rds_password
등과 같은 민감한 데이터는 환경 변수나 별도의 파일 등을 통해 관리하면 보안에 더 유리할 것입니다. 이러한 정보가 코드에 포함되어 있으면, 코드를 공유하거나 저장할 때 보안 위험이 발생합니다. -
RDS 비밀번호:
ncp_rds_password
의 값은 강력한 비밀번호 생성 규칙을 따르는 것이 좋습니다. 대문자, 소문자, 숫자 및 특수 문자와 같이 다양한 형태의 문자들을 포함하는 것이 이상적입니다. -
기본값: 'default'로 지정된 값을 재검토하십시오. 예를 들어, "KR"이 해당 서비스에 필요한 모든 상황에 적용되는지, 아니면 사용자가 이 값을 직접 설정해야 하는 경우에는 기본값 설정을 제거하는 것이 좋습니다.
-
주석: 변수마다 설명이 주석으로 달려 있는데, 이를 통해 코드의 가독성을 향상시킬 수 있습니다. 가능한 한 자세하고 명확하게 설명하면 코드를 보는 다른 사람들에게 도움이 됩니다.
모든 소프트웨어 프로젝트에서 코딩 표준과 가이드라인을 확립하는 것이 중요하며, 이러한 기준은 일관성을 유지하고 높은 품질의 코드를 작성하는 데 도움이 됩니다.
- name: Terraform Apply | ||
working-directory: ${{ env.working-directory }} | ||
if: github.ref == 'refs/heads/infra/main' && github.event_name == 'push' | ||
run: terraform apply -auto-approve -var=ncp_access_key=${{ secrets.NCP_ACCESS_KEY_ID }} -var=ncp_secret_key=${{ secrets.NCP_SECRET_ACCESS_KEY }} -input=false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드 패치는 GitHub Actions를 사용하여 Terraform을 이용한 Infrastructure as Code (IaC) 환경 구성을 하는 것입니다. 다음은 제 기준에서 몇 가지 주요 포인트들입니다:
-
"Terraform Setup" 단계에서는
${{ secrets.TF_API_TOKEN }}
토큰을 사용하여 Terraform CLI에 credentials를 설정하고 있습니다. 이 민감한 정보들이 적절히 관리되고 있는지 확인하세요. -
"Terraform Plan" 및 "Terraform Apply" 단계에서는 또다른 민감한 정보인
${{ secrets.NCP_ACCESS_KEY_ID }}
및${{ secrets.NCP_SECRET_ACCESS_KEY }}
을 사용합니다. -
Terraform 버전(1.8.2)이 코드와 호환되는지 검사해보세요. 때로는 Terraform의 새 버전에서 발생하는 변경 사항이 기존 코드와 문제를 일으킬 수 있기 때문입니다.
-
마지막으로, "push" 이벤트에 대한 처리와 관련된 로직(
if: github.ref == 'refs/heads/infra/main' && github.event_name == 'push'
)이 흐름에 적절한지 확인해 보세요. 이것은 실수로 다른 branch에 push될 경우나 다른 이벤트 발생 시에 terraform apply가 실행되지 않도록 방지할 수 있습니다. -
가능성은 비교적 낮지만, Terraform 실행 중 운영 시스템에서 문제가 발생할 수 도 있으니,
terraform plan
의 출력 결과를 체크해서 보일러 플레이트나 예상치 못한 변경 사항이 존재하지 않는지 확인해야 합니다. -
한 가지 가능한 개선 사항은 "Terraform Format" 단계에서
-check
옵션을 사용하여 코드 형식 검사를 수행하고 있습니다. 이 부분은 별도의 CI 단계로 나눠서 더 이른 시점에 format 문제를 발견하면 좋을 것 같습니다.
기본적으로는 잘 작성된 것 같지만 세부적인 설정과 비즈니스 요구에 따라 다른 수정 및 유지관리가 필요할 수 있습니다.
type = string | ||
default = "thisisrdspassword@1" | ||
description = "The password for the RDS instance" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 코드는 Terraform 설정파일에서 사용되는 변수를 선언하는 부분인 것 같습니다. 몇 가지 코멘트를 추가하고 싶네요:
-
버그 위험: 민감한 정보들(
access_key
,secret_key
,rds_password
)은 직접 코드에 포함시키는 것보다는 외부에서 환경 변수 등을 통해 주입收게 하는 편이 안전합니다. 공개된 소스코드에 이러한 정보가 포함되면 보안 위협이 될 수 있습니다. -
개선 제안: "rds_password"와 같이 default 값으로 비밀번호를 설정하는 것은 좋지 않습니다. 보안상의 문제가 될 수 있기 때문입니다.
-
개선 제안: 모든 변수에 대한 description이 잘 작성되어 있는데, 이 내용은 가능한 한 구체적이고 이해하기 쉽게 작성하는 것이 좋습니다.
-
개선 제안: 'region'의 기본값이 "KR"로 설정되어 있음. 이것도 필수 입력항목으로 바꾸면 배포 지역 선택에 유연성이 생길 것입니다.
-
최선의 사례: 이 코드는 함수, 클래스 등의 주석을 달아야 하는 경우가 아니므로, 충분히 이해할 수 있다고 판단됩니다.
🎫 연관 이슈
resolved: #7 테라폼을 활용하여 인프라를 구성합니다.
💁♂️ PR 내용
Terraform을 활용하여 NCP 인프라를 구성하였습니다.
포크(fork)된 곳 에서의 PR은 액션이 정상적으로 동작하지 않아 다시 PR 합니다.
🙏 작업
vpc
를 구성합니다.server
인스턴스를 구성합니다.acg(aws/securitygroup)
을 구성합니다.database
를 구성합니다.loadbalancer
를 구성합니다.🙈 PR 참고 사항
init script
의 경우 현재 nginx를 임의로 동작하도록 구성object store
의 경우 NCP 테라폼에서 아직 지원하지 않아 직접 구성하여야 합니다.database
의 경우 변경 사항이 생길 경우 수정app.terraform.io
를 통해 클라우드에서 테라폼 관련 기록을 관리하고 있습니다. (프리티어가 있어 무료로 사용가능)NCP
역시 @belljun3395 것을 사용하는 중📸 스크린샷
🤖 테스트 체크리스트