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

Add security settings #25

Merged
merged 17 commits into from
Sep 28, 2023

Conversation

dmitry-ratushnyy
Copy link
Contributor

@dmitry-ratushnyy dmitry-ratushnyy commented Sep 18, 2023

The goal of this PR is:

1.Add required security Manifests to ROCK
2. Remove util-linux deb since it is packaged in the snap that the ROCK is based off

@dmitry-ratushnyy dmitry-ratushnyy force-pushed the dmitry.ratushny/add_security_manifest branch 2 times, most recently from 8aaf6a8 to e0c841d Compare September 19, 2023 15:22
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the dmitry.ratushny/add_security_manifest branch from e0c841d to 97dab0f Compare September 19, 2023 15:38
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the dmitry.ratushny/add_security_manifest branch from a695c57 to c64c26d Compare September 19, 2023 16:20
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the dmitry.ratushny/add_security_manifest branch from 4bb18a7 to a6f71ba Compare September 25, 2023 14:18
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the dmitry.ratushny/add_security_manifest branch from a6f71ba to 5ba423f Compare September 25, 2023 14:26
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the dmitry.ratushny/add_security_manifest branch from 7f63791 to 8cdea45 Compare September 26, 2023 16:49
@dmitry-ratushnyy dmitry-ratushnyy changed the title [WIP] Add security settings Add security settings Sep 27, 2023
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the dmitry.ratushny/add_security_manifest branch from 5c4a33f to 7ac4ac4 Compare September 27, 2023 09:21
@dmitry-ratushnyy dmitry-ratushnyy force-pushed the dmitry.ratushny/add_security_manifest branch from 7ac4ac4 to bbff857 Compare September 27, 2023 09:35
@dmitry-ratushnyy dmitry-ratushnyy requested review from jardon and removed request for jardon September 28, 2023 05:10
@@ -16,6 +16,7 @@ jobs:
run: |
sudo snap install yq
sudo snap install rockcraft --classic --edge --revision=687
sudo snap install charmcraft --classic --revision 1349
Copy link
Contributor

Choose a reason for hiding this comment

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

Noob question: why different charmcraft here than in integration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK it is actually the same version, but in this case we install it while in case of integration test we update the installed one

# enable security monitoring
rocks=usr/share/rocks/
mkdir -p ${rocks}
## for deb packages
Copy link
Contributor

Choose a reason for hiding this comment

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

nit can we have enters after the comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can try but linter was complaining about those

Copy link
Contributor

@MiaAltieri MiaAltieri Sep 28, 2023

Choose a reason for hiding this comment

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

hmm maybe we can copy the linter from opensearch? they seem to have less problems, and then you could have the dpkg command the way you originally intended?

(doesn't have to be now) but would be good long term if we had our linters alligned across products on the NoSQL team

Copy link
Contributor Author

@dmitry-ratushnyy dmitry-ratushnyy Sep 28, 2023

Choose a reason for hiding this comment

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

looks like opensearch rock does not have a linter (if I am not missing anything)

rockcraft.yaml Show resolved Hide resolved
@MiaAltieri MiaAltieri self-requested a review September 28, 2023 11:35
@dmitry-ratushnyy dmitry-ratushnyy merged commit 2e52a77 into 5-22.04 Sep 28, 2023
5 checks passed
@dmitry-ratushnyy dmitry-ratushnyy deleted the dmitry.ratushny/add_security_manifest branch September 28, 2023 11:47
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.

3 participants