-
Notifications
You must be signed in to change notification settings - Fork 23
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
chore(prometheus): unsafe deserialisation #300
Conversation
@pytest.fixture() | ||
def AnsibleDefaults(): | ||
with open("defaults/main.yml", 'r') as stream: | ||
return yaml.safe_load(stream) |
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.
this is the actual change, the rest are just files been added to the repository
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.
outdated, we are removing this submodule since we do not use this submodule for our prometheus setup in l-cloud-gcc
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.
did a quick check. l-cloud-gcc
doesn’t use the prometheus
module from terraform-modules
which uses the roles/prometheus
submodule. It is ok to remove but I would suggest to remove the whole terraform-modules/modules/prometheus
including the roles/prometheus
submodule instead of just the role submodule alone. This is because terraform-modules
is a public repo, removing the role submodule alone will make the whole prometheus
module incomplete & unusable. We are unsure if anyone using it & it is not obvious if it encounters errors due to this missing role submodule 🙏
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.
Looks good! Please remove all the references to prometheus (just some remaining mainly in grafana, telegraf & traefik modules) 🙏
For the remaining reference, there is no need to remove as they are independent from the prometheus module (can see that no outputs from prometheus module is reference in the other modules) the remaining reference are variables for users to link up their prometheus client based on the address / port / nomad consul names which are not reliant on the module, so I would rather not remove them |
Story
https://www.notion.so/aipf-locus/Unsafe-Deserialization-1134e47c1d87808792d1eaaff7471966?pvs=4
What Changed
yaml.safe_load
instead ofyaml.load
so that the deserialisation is safe.