로컬 테스트 방식 변경 및 일부 파이프라인 일부 수정

jongwoo328 commented 2 months ago

Done

aws sam template 추가
- sam으로 lambda와 동일한 환경에서 로컬 테스트 가능
real이 아닌 경우 메시지 보내지 않도록 코드 수정 (임시)
github workflow 수정 (테스트 필요)
- 개발용 workflow 분리
- job 분리하여 관련없는 작업은 병렬실행가능하도록 수정
- 테스트 후 수정필요할 수 있음

To Reviewers

적용방법
- aws sam 설치
- aws configure로 profile 설정하기
- jetbrain ide 사용할경우
  - aws toolkit 설치
  - template에서 실행버튼 클릭
  - runtime은 node18.x, event는 template에서 Scheduled Event, AWS connection에서 profile 지정
  - 실행
- jetbrain ide 사용하지 않는 경우
  - sam build NewsScraperDevelop --template {프로젝트위치}lambdas/news-scraper/template.yaml --build-dir {프로젝트위치}lambdas/news-scraper/.aws-sam/build 로 빌드
  - sam local invoke NewsScraperDevelop로 실행
현재 한계점
- 코드 변경된경우 sam build가 필요한데 이 때 시간이 오래걸림
- 가능한 방안
  1. 그대로 사용
  2. 배포방식을 docker image -> esbuild를 통한 js파일 업로드로 변경
    - esbuild로 번들링하면 1MB 미만의 js 코드로 번들링 가능
    - 1MB 미만의 단일 js 파일이므로 바로 업로드 가능. 람다 업로드 시 인라인코드 제한 3MB이라고 함
    - 예시
```
esbuild src/index.ts \
--bundle \
--minify \
--target=es2020 \
--sourcemap \
--external:@sparticuz/chromium \
--external:puppeteer-core \
--outfile=dist/index.js
```
    - 다만 이렇게 되면 common/config 에서 .env를 읽어오는 방식과 혼용이 불가능 (.env가 같이 번들링되지않기 때문)
    - 이 방식을 적용한다면, github action 실행 시점에 env를 github secret에서 주입해야할 듯
    - 예시
```
esbuild src/index.ts \
--bundle \
--minify \
--target=es2020 \
--sourcemap \
--external:@sparticuz/chromium \
--external:puppeteer-core \
--define:process.env.~~~="${{ secrets.~~~ }}" \
--outfile=dist/index.js
```

hyejungg commented 2 months ago

코드 리뷰 전에 PR 내용 바탕으로 질문 먼저 드립니다.

1.

1MB. 미만의 단일 js 파일이므로 바로 업로드 가능. 람다 업로드 시 인라인코드 제한 3MB이라고 함

esbuild 방식은 현재보다 프로젝트 규모가 커질 경우 (1mb 이상인 경우) esbuild 로 인라인코드 업로드는 불가한거죠 ?
그렇다면 현재는 무리없이 1mb 미만이더라도 추후에 1mb 이상이 될 때 다른 배포 방식(현재 docker 처럼)으로 전환해야 하는 불편함이 있을 것 같아 현재 배포 방식을 유지하면 좋을 것 같습니다.
동적으로 파일 크기에 따라 배포 방식을 다르게 하는 방법도 있을 것 같은데 ... 개인적으로 좋아보이진 않네요 ...

2. 현재 PR은 현재 배포 방식을 유지한 상태인가요?

3. lambda 에서 사용하고 있는 Puppeteer 버전과 로컬에서 개발 시 사용하는 puppeteer 버전의 차이로 인해 개발에 어려움이 있습니다. 해당 부분은 해결된건가요?

43 해당 이슈 확인하느라 로컬에서 돌려보는데 문제가 많더라구요.
버전을 맞추는게 우선이 되어야 할 것 같은데 해결됐다면 다행입니다 ....

jongwoo328 commented 2 months ago

그리고 배포 테스트 겸 설정파일 생성을위해서 sam deploy --guided를 로컬에서 실행했는데

Error: Failed to create managed resources: An error occurred (AccessDenied) when calling the CreateChangeSet operation: User: arn:aws:iam::339712918956:user/justin.dev is not authorized to perform: cloudformation:CreateChangeSet on resource: arn:aws:cloudformation:ap-northeast-2:339712918956:stack/aws-sam-cli-managed-default/* because no identity-based policy allows the cloudformation:CreateChangeSet action

이렇게 실패하네요.. 권한 받을 수 있을까요?

jongwoo328 commented 2 months ago

코드 리뷰 전에 PR 내용 바탕으로 질문 먼저 드립니다.

1.

1MB. 미만의 단일 js 파일이므로 바로 업로드 가능. 람다 업로드 시 인라인코드 제한 3MB이라고 함

esbuild 방식은 현재보다 프로젝트 규모가 커질 경우 (1mb 이상인 경우) esbuild 로 인라인코드 업로드는 불가한거죠 ?

그렇다면 현재는 무리없이 1mb 미만이더라도 추후에 1mb 이상이 될 때 다른 배포 방식(현재 docker 처럼)으로 전환해야 하는 불편함이 있을 것 같아 현재 배포 방식을 유지하면 좋을 것 같습니다.

동적으로 파일 크기에 따라 배포 방식을 다르게 하는 방법도 있을 것 같은데 ... 개인적으로 좋아보이진 않네요 ...

2. 현재 PR은 현재 배포 방식을 유지한 상태인가요?

3. lambda 에서 사용하고 있는 Puppeteer 버전과 로컬에서 개발 시 사용하는 puppeteer 버전의 차이로 인해 개발에 어려움이 있습니다. 해당 부분은 해결된건가요?

동작하지않는 파싱 사이트 수정 #43 해당 이슈 확인하느라 로컬에서 돌려보는데 문제가 많더라구요.

버전을 맞추는게 우선이 되어야 할 것 같은데 해결됐다면 다행입니다 ....

👌
현재까지는 배포를 수정하진 않았고 배포 테스트를 위해서 위에 작성한 대로 권한이 필요한 상태입니다. sam으로 배포까지 하는걸로 저번에 얘기했던걸로 기억해서 그대로 진행하려고요
이건 고려 못했던 부분인데 커밋 추가했습니다. puppeteer-core 말고 puppeteer만 사용하도록 수정했고, 아직 phase? stage? 어떤 변수를 쓸지 확정이 안됐지만 어쨌든 실제환경에서는 @sparticuz/chromium의 크로미움을, 로컬에서는 pupetter의 크로미움을 사용하도록 수정했습니다. 둘 다 버전은 127로 맞춰지도록 puppeteer 버전도 맞춰놨습니다.

hyejungg commented 2 months ago

그리고 배포 테스트 겸 설정파일 생성을위해서 sam deploy --guided를 로컬에서 실행했는데

Error: Failed to create managed resources: An error occurred (AccessDenied) when calling the >CreateChangeSet operation: User: arn:aws:iam::339712918956:user/justin.dev is not authorized to >perform: cloudformation:CreateChangeSet on resource: arn:aws:cloudformation:ap-northeast->2:339712918956:stack/aws-sam-cli-managed-default/* because no identity-based policy allows the >cloudformation:CreateChangeSet action

이렇게 실패하네요.. 권한 받을 수 있을까요?

우선 cloudformation full access 권한 추가했습니다.

puppeteer 맞춰주셔서 감사합니다 🥹

jongwoo328 commented 2 months ago

실행가능하도록 코드 수정했습니다. 크롬 127 버전이 AmazonLinux2 랑 안맞는다고 해서 수정하면서 AmazonLinux2023 기반으로 하는 node20으로 그냥 올려버렸어요..

그리고 빌드가좀 오래걸리다 보니 빨리빨리 확인해야할때는 코드로 직접 실행하고 코드가 어느정도 안정화되면 로컬에서 이미지로 테스트 하시는게 좋을것같아요

지금도 news-scraper에 들어가서 node_modules/.bin/ts-node src/local.ts 로 실행하면 로컬기준으로 코드 실행할 수 있거든요

jongwoo328 commented 2 months ago

sam으로 배포하면서 cloudformation으로 리소스 자동생성해서 배포하게되는데 role 생성 권한이 없어서 실패했는데 이거 봐주실 수 있을까요

"User: arn:aws:iam::339712918956:user/justin.dev is not authorized to perform: iam:CreateRole on resource: arn:aws:iam::339712918956:role/news-scraper-NewsScraperDevelopRole-doRdktbBOTua because no identity-based policy allows the iam:CreateRole action (Service: Iam, Status Code: 403, Request ID: e321dd19-2cd8-4088-b41b-55200ad1e439)"" (RequestToken: b4d07ef1-ebd1-d124-98a1-3c7230402aca, HandlerErrorCode: UnauthorizedTaggingOperation)

관련로그는 cloudformation에서 news-scraper에서 보실 수 있어요

hyejungg commented 2 months ago

sam으로 배포하면서 cloudformation으로 리소스 자동생성해서 배포하게되는데 role 생성 권한이 없어서 실패했는데 이거 봐주실 수 있을까요

"User: arn:aws:iam::339712918956:user/justin.dev is not authorized to perform: iam:CreateRole on resource: arn:aws:iam::339712918956:role/news-scraper-NewsScraperDevelopRole-doRdktbBOTua because no identity-based policy allows the iam:CreateRole action (Service: Iam, Status Code: 403, Request ID: e321dd19-2cd8-4088-b41b-55200ad1e439)"" (RequestToken: b4d07ef1-ebd1-d124-98a1-3c7230402aca, HandlerErrorCode: UnauthorizedTaggingOperation)

관련로그는 cloudformation에서 news-scraper에서 보실 수 있어요

그런데 왜 해당 권한이 필요한건지 이해가 잘 안가는데요 ... ! 설명해주실 수 있을까요 ?!

jongwoo328 commented 2 months ago

왜냐하면 리소스를 생성할때 해당 리소스에 대한 권한도 같이 생성하기 때문입니다

jongwoo328 commented 2 months ago

아니면 권한을 따로 만들어놓고 자동생성이아니라 이미 존재하는 권한 참조하게 할 수도 있다고 하네요

hyejungg commented 2 months ago

저스틴은 어떤 방식으로 하고 싶으신가요 ??

jongwoo328 commented 2 months ago

둘 다 괜찮을 것 같아요. 편한건 자동생성되는 방식이긴 한데 별도로 권한을 만들고 사용하는 방식으로 해도 상관 없습니다.

만약에 별도 권한을 참조해서 쓴다고 하면 배포 테스트 하고 설정하는 동안에는 아마 한번에 안될 것 같아서 구성하는 동안은 권한을 받고 자동 생성된 권한을 사용하고 마지막에 그 내용과 동일하게 별도로 생성한 권한을 적용하도록 해야할 것 같습니다

hyejungg commented 2 months ago

이미 createRole권한은 있는데, service-role에만 적용되어 있어서 필요한 리소스를 추가했어요.
그런데 리소스 이름이 NewsScraperDevelopRole-*처럼 자동으로 생성되다 보니 이름이 계속 바뀌는 것 같아요. 그래서 와일드카드(*)를 사용해서 범위를 넓혀야 할 것 같아서 일단 리소스 전체를 허용하는 방식으로 추가해두었습니다.

그런데 개인적인 생각으로는 이렇게 배포 테스트 중에도 한 번에 해결되지 않을 수 있을 것 같아서, 지금부터 그냥 별도의 권한을 만들어서 사용하는 건 어떨까 싶어요. 왜냐하면 이전에 시간이 없어서 권한 정리를 하려고 했었는데 못했던 부분도 있고, 당시에 빠른 개발을 위해서 여러 권한을 *로 열어두었는데 나중에 정리할 때도 정확히 어떤게 필요했는지 파악하기 어려워서 정리를 못할 것 같다는 느낌도 있습니다 ..

그런데 저스틴 개발하실 때 불편하실 것 같으면 그냥 하시던대로 하셔도 됩니다!

jongwoo328 commented 2 months ago

그럼 시간이 좀 걸리더라도 정리하면서 하시죠

지금 구조를 제가 조회가 안돼서 잘 모르겠지만 약간 생각해봤는데 대략적으로 이런 방향이 어떨까요?

아무래도 이거 작업하는 동안은 딱 필요한 권한만 유지하기는 쉽지 않아 보이긴 합니다 결국 다 끝나고 정리하는 작업은 한번 필요할 것 같아요

사용자	설명	내용
github action (가칭)	github action에 부여할 user	github action에서만 사용하는 용도, 배포에 필요한 권한만 할당 (작업 및 테스트 끝난 후 최종적으로 필요한 권한만 수동으로 뷰요)
justin	-	배포 파이프라인 완성전 까지는 람다 role에 권한부여 권한을 직접 받거나 molly에게 특정 리소스에 필요한 권한 부여 요청, 나중에 구성완료 후 리소스에 프로젝트 태그 지정하면 불필요한 권한 제거 및 필요한 리소스에 대해 태그 기반 조회권한만 남김

hyejungg commented 2 months ago

넵 좋습니다. 현재는 모든 사용자가 같은 권한과 같은 정책을 바라보고 있는데 유저별로 다르게 두는게 맞는 것 같네요 ㅠㅠ

현재까지 콘솔에서 확인된 내용을 공유드리자면 다음과 같습니다.

- 사용자 : AI_NEWS_TREND_BOT_DEPLOY (배포 시 사용)
- 사용자 : justin.dev
- 사용자 : molly.ouo
3개의 사용자가 있고, AI_NEWS_NOTI_BOT 이라는 사용자 그룹으로 묶여 있습니다.

justin.dev, molly.ouo는 어드민 느낌으로 모든 권한을 갖도록 하는 건 괜찮은데 저 배포용 친구의 권한들을 정리 한 번 하면 좋을 것 같네요 .. 이건 같이 화면 보면서 하는게 좋을 것 같네요 ..

jongwoo328 commented 2 months ago

일단 제가 개발하는데는 자동생성으로 해놓고 최종적으로 자동생성된 role에는 필요한 권한만 부여되어있을테니 그것 보고 동일하게 role 생성한다음에 최종으로 자동생성에서 role 지정방식으로 변경하는게 편할 것 같아요 그리고 제가 권한 부여 권한까지 있으면 더 좋을 것 같긴한데 그렇게 할지, 아님 매번 요청하는것으로 할지도 한번 생각해주세요 +) 이번엔 또 iam:TagRole 가 없다고 실패했네요 ㅠ

자세한건 내일이나 그 뒤에 직접 보고 얘기하시죠

hyejungg / ai-news-noti-bot