Skip to content
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

Corrections for TF manifest #4

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mikhaildoc-documentat
Copy link

No description provided.

resource "yandex_iam_service_account" "sa-for-obj-storage" {
folder_id = local.folder_id
name = local.os_sa_name
}

# Grant the service account storage.admin role to create storages and grant bucket ACLs.
# Grant the service account storage.admin role to create storages and grant bucket ACLs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Grant the service account storage.admin role to create storages and grant bucket ACLs
# Grant the service account storage.admin role to manage buckets and grant bucket ACLs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Предлагаю здесь поправить.
Роль позволяет не только создавать, но и в целом управлять бакетом, в том числе и удалять его. Ну и термин "бакет" сюда больше подходит, чем хранилище.

resource "yandex_iam_service_account" "sa-for-obj-storage" {
folder_id = local.folder_id
name = local.os_sa_name
}

# Grant the service account storage.admin role to create storages and grant bucket ACLs.
# Grant the service account storage.admin role to create storages and grant bucket ACLs
resource "yandex_resourcemanager_folder_iam_binding" "s3-editor" {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
resource "yandex_resourcemanager_folder_iam_binding" "s3-editor" {
resource "yandex_resourcemanager_folder_iam_binding" "s3-admin" {

resource "yandex_storage_bucket" "input-bucket" {
access_key = yandex_iam_service_account_static_access_key.sa-static-key.access_key
secret_key = yandex_iam_service_account_static_access_key.sa-static-key.secret_key
bucket = local.input_bucket

depends_on = [
yandex_resourcemanager_folder_iam_binding.s3-editor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
yandex_resourcemanager_folder_iam_binding.s3-editor
yandex_resourcemanager_folder_iam_binding.s3-admin

resource "yandex_storage_bucket" "output-bucket" {
access_key = yandex_iam_service_account_static_access_key.sa-static-key.access_key
secret_key = yandex_iam_service_account_static_access_key.sa-static-key.secret_key
bucket = local.output_bucket

depends_on = [
yandex_resourcemanager_folder_iam_binding.s3-editor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
yandex_resourcemanager_folder_iam_binding.s3-editor
yandex_resourcemanager_folder_iam_binding.s3-admin

@@ -197,7 +212,7 @@ resource "yandex_dataproc_cluster" "dataproc-cluster" {

hadoop {
services = ["HDFS", "SPARK", "YARN"]
ssh_public_keys = [file(local.dp_ssh_key)]
ssh_public_keys = ["${file(local.dp_ssh_key)}"]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Здесь было правильно. Давай вернём как было.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут используется не явно прописанный путь, а локальная переменная, с ней путь нужно записывать именно так

Copy link

@Odyvia42 Odyvia42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Давай ещё добавим в кластер зависимости от ресурсов с ролями dataproc.agent и dataproc.provisioner, как в других манифестах.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants