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

Mlo 133 create admission controller #906

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

parikls
Copy link

@parikls parikls commented Feb 12, 2025

No description provided.

Copy link

linear bot commented Feb 12, 2025

Copy link

gitguardian bot commented Feb 12, 2025

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@github-actions github-actions bot added the review-ready PR is ready for the review label Feb 12, 2025
@parikls parikls marked this pull request as draft February 12, 2025 10:41
@github-actions github-actions bot removed the review-ready PR is ready for the review label Feb 12, 2025
@parikls parikls force-pushed the MLO-133-create-admission-controller branch from e7451fe to 2767261 Compare February 12, 2025 11:21
LABEL_APOLO_ORG_NAME = "platform.apolo.us/org"
LABEL_APOLO_PROJECT_NAME = "platform.apolo.us/project"
LABEL_APOLO_STORAGE_MOUNT_PATH = "platform.apolo.us/storage/mountPath"
LABEL_APOLO_STORAGE_HOST_PATH = "platform.apolo.us/storage/hostPath"
Copy link
Author

Choose a reason for hiding this comment

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

@zubenkoivan this supposed to be a storage URL, e.g., storage://org/proj/path/to/dir. Maybe hostPath label name is not the best option here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a single annotation to pass storage config? we can put json object there

@parikls parikls requested a review from zubenkoivan February 12, 2025 14:16
@parikls parikls marked this pull request as ready for review February 12, 2025 14:17
@github-actions github-actions bot added the review-ready PR is ready for the review label Feb 12, 2025
@@ -55,6 +55,16 @@ spec:
{{- end }}
env:
{{- include "platformStorage.env" . | nindent 10 }}
- name: NP_STORAGE_API_K8S_API_URL
Copy link
Contributor

Choose a reason for hiding this comment

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

I think main api doesn't use kube client

LABEL_APOLO_ORG_NAME = "platform.apolo.us/org"
LABEL_APOLO_PROJECT_NAME = "platform.apolo.us/project"
LABEL_APOLO_STORAGE_MOUNT_PATH = "platform.apolo.us/storage/mountPath"
LABEL_APOLO_STORAGE_HOST_PATH = "platform.apolo.us/storage/hostPath"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a single annotation to pass storage config? we can put json object there

ca_data = Path(ca_path).read_text() if ca_path else None

token_path = self._environ.get("NP_STORAGE_API_K8S_TOKEN_PATH")
token = Path(token_path).read_text() if token_path else None
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't apolo-kube-client read ca_data and token from files?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-ready PR is ready for the review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants