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

DRAFT : Feat/add quarentine bucket #3

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

Rkejji
Copy link
Contributor

@Rkejji Rkejji commented May 2, 2023

No description provided.

@Rkejji Rkejji requested review from AlexisVLRT and pe-artefact May 2, 2023 16:05
@@ -11,6 +11,12 @@ This Terraform module allows you to configure and deploy a data lake with:
- Quarantine bucket
...

## Requirements:
User or service account credentials with the following roles must be used to set the IAM policies to the resources:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
User or service account credentials with the following roles must be used to set the IAM policies to the resources:
Roles required to deploy this module:

Copy link
Collaborator

Choose a reason for hiding this comment

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

À supprimer

Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo: quarantine, not quarentine

Copy link
Collaborator

Choose a reason for hiding this comment

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

Lint the python code with something like flake8 please

return data["attributes"]

def move_object_to_quarentine(bucket_name : str, object_name : str ) -> None:
logging.warn(f"Will move the object {object_name} to the quarentine bucket {destination_bucket} ")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ça fonctionne ça ? J'ai l'impression que destination_bucket est pas encore défini ligne 39.

Copy link
Collaborator

Choose a reason for hiding this comment

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

De plus, si le bucket de quarantaine est le même bucket que le bucket où a été posé le fichier initialement, le message risque de pas être clair. Je pense que "Moving file {object_name} to quarantine {destination_bucket}/{quarantine_path}" serait plus compréhensible.

custom_attributes = {
regex = var.object_validation_regex # "^([a-z]{2}\\d{3}|[a-z]{3}\\d{2})-[a-z]{2}-\\d{2}$"
}
event_types = ["OBJECT_FINALIZE", "OBJECT_DELETE", "OBJECT_ARCHIVE", "OBJECT_METADATA_UPDATE"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Enlever OBJECT_ARCHIVE

data "google_storage_project_service_account" "gcs_account" {
}

resource "google_pubsub_topic_iam_binding" "bind_gcs_svc_acc" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

member

Copy link
Collaborator

Choose a reason for hiding this comment

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

run terraform fmt

Comment on lines +55 to +59
variable "storage_admins" {
description = "Liste of members that can create/delete/edit a bucket"
type = list(string)
default = []
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pas dans le scope de ce module

Comment on lines +61 to +71
variable "object_admins" {
description = "Liste of members that can create/delete an object"
type = list(string)
default = []
}

variable "object_viewers" {
description = "List of members that can view objects"
type = list(string)
default = []
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Il faut pouvoir donner des rôles par bucket, plutôt que par landing zone, je pense.

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

Successfully merging this pull request may close these issues.

2 participants