Skip to content

feat(chart): add security context on container levels - #25985

Closed
anja-istenic wants to merge 8 commits into
apache:mainfrom
anja-istenic:add-container-security-ctx
Closed

feat(chart): add security context on container levels#25985
anja-istenic wants to merge 8 commits into
apache:mainfrom
anja-istenic:add-container-security-ctx

Conversation

@anja-istenic

Copy link
Copy Markdown
Contributor

It is recommended practice to give a resource (pod/container) the least privileges it needs. The Airflow Helm Chart already supports configuring the security context on the pod level. This PR adds the securityContext configurations on the container level (more info on container level securityContext here).

This change is fully backwards compatible, as the securityContext value configuration is optional and defaults to {}


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Also a chapter was added to recommend taking a backup before
the migration.

Based on discussions and user input from apache#25866, apache#24526

Closes: apache#24526

Improve cleanup of temporary files in CI (apache#25957)

After recent change in Paralell execution, we start to have
infrequent "no space left on device" message - likely caused by
the /tmp/ generated files clogging the filesystem from multiple
runs. We could fix it by simply running cleanup after parallel
job always, but this is not good due to diagnostics needed
when debugging parallel runs locally so we need to have
a way to skip /tmp files deletion.

This PR fixes the problem twofold:

* cleanup breeze instructions which is run at the beginning of
  every job cleans also /tmp file
* the parallel jobs cleans after themselvs unless skipped.

Properly check the existence of missing mapped TIs (apache#25788)

The previous implementation of missing indexes was not correct. Missing indexes
were being checked every time that `task_instance_scheduling_decision` was called.
The missing tasks should only be revised after expanding of last resort for mapped tasks have been done. If we find that a task is in schedulable state and has already been expanded, we revise its indexes and ensure they are complete. Missing indexes are marked as removed.
This implementation allows the revision to be done in one place

Co-authored-by: Tzu-ping Chung <uranusjr@gmail.com>

Fix dataset_event_manager resolution (apache#25943)

Appears `__init__` is not invoked as part of `_run_raw_task` due to the way TI is refreshed from db.  Centralize dataset manager instantiation instead.

Fix unhashable issue with secrets.backend_kwargs and caching (apache#25970)

Resolves apache#25968

Fix response schema for list-mapped-task-instance (apache#25965)

update areActiveRuns, fix states (apache#25962)
@anja-istenic
anja-istenic force-pushed the add-container-security-ctx branch from 28f7d99 to 5d5fe14 Compare August 29, 2022 06:22
@anja-istenic

Copy link
Copy Markdown
Contributor Author

@dstandish @jedcunningham would appreciate any feedback on this PR :)

@anja-istenic

Copy link
Copy Markdown
Contributor Author

@potiuk @jedcunningham @dstandish @kaxil any feedback or review would be much appreciated

@potiuk

potiuk commented Sep 10, 2022

Copy link
Copy Markdown
Member

This looks good to me, but I woudl love the others to take a look too :)

@jedcunningham

Copy link
Copy Markdown
Member

(I'd argue this is a duplicate of #24588)

@potiuk

potiuk commented Sep 18, 2022

Copy link
Copy Markdown
Member

(I'd argue this is a duplicate of #24588)

So I guess we need to suggest some way forward @jedcunningham :) to @ChrisFraun and @anja-istenic - which one should we move forward? Or maybe @ChrisFraun and @anja-istenic you can review each other's PR and agree between yourselves which one should proceed (either of the two needs rebase to follow).

@anja-istenic

Copy link
Copy Markdown
Contributor Author

So I guess we need to suggest some way forward @jedcunningham :) to @ChrisFraun and @anja-istenic - which one should we move forward? Or maybe @ChrisFraun and @anja-istenic you can review each other's PR and agree between yourselves which one should proceed (either of the two needs rebase to follow).

Thanks all for reviewing this. While I was out @ChrisFraun added the extra changes from this PR to #24588, I'm therefore closing this now redundant PR.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants