Air-gapped environment #9189
Draft
Air-gapped environment #9189
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ytimocin
temporarily deployed
to
functional-tests
GitHub Actions
Inactive
|
The gist that installs Radius on an air-gapped cluster: https://gist.github.com/ytimocin/8887d95ab1409562f4646fd30edb101c. Still WIP and needs to be updated. |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
| commonflags.AddKubeContextFlagVar(cmd, &runner.KubeContext) | ||
| cmd.Flags().BoolVar(&runner.Reinstall, "reinstall", false, "Specify to force reinstallation of Radius") | ||
| cmd.Flags().StringVar(&runner.Chart, "chart", "", "Specify a file path to a helm chart to install Radius from") | ||
| cmd.Flags().StringVar(&runner.ContourChart, "contour-chart", "", "Specify a local file path to a helm chart to install Contour from") |
There was a problem hiding this comment.
do these have to be a local filepath or can they be an OCI registry (e.g. on a local CR)
There was a problem hiding this comment.
local path for now
| options.Radius.ChartPath = cliOptions.Radius.ChartPath | ||
| } | ||
|
|
||
| if cliOptions.Radius.ChartRepo != "" { |
There was a problem hiding this comment.
was this missing before?
pkg/cli/helm/contourclient.go
Outdated
| if err != nil { | ||
| return fmt.Errorf("failed to get contour chart, err: %w, helm output: %s", err, helmOutput.String()) | ||
| // Use provided repo URL if available, otherwise use default | ||
| repo := options.ChartRepo |
There was a problem hiding this comment.
can probably remove this codeblock, looks like dupe of below
ytimocin
force-pushed
the
ytimocin/air-gapped-cluster
branch
from
ec13934 to
3bacc6f
Compare
ytimocin
had a problem deploying
to
functional-tests
GitHub Actions
Failure
Unit Tests3 959 tests +106 3 955 ✅ +106 9m 27s ⏱️ + 2m 22s Results for commit c847ea5. ± Comparison against base commit e54aef5. This pull request removes 84 and adds 190 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
Nice, LGTM. This will be a one off or would we officially support these options in main? |
ytimocin
force-pushed
the
ytimocin/air-gapped-cluster
branch
from
3bacc6f to
0fef950
Compare
ytimocin
had a problem deploying
to
functional-tests
GitHub Actions
Failure
ytimocin
force-pushed
the
ytimocin/air-gapped-cluster
branch
from
0fef950 to
205332c
Compare
ytimocin
temporarily deployed
to
functional-tests
GitHub Actions
Inactive
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9189 +/- ##
==========================================
+ Coverage 49.36% 49.37% +0.01%
==========================================
Files 636 640 +4
Lines 48757 50612 +1855
==========================================
+ Hits 24067 24992 +925
- Misses 22860 23669 +809
- Partials 1830 1951 +121 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ytimocin
force-pushed
the
ytimocin/air-gapped-cluster
branch
from
205332c to
edb52e7
Compare
ytimocin
had a problem deploying
to
functional-tests
GitHub Actions
Failure
ytimocin
had a problem deploying
to
functional-tests
GitHub Actions
Failure
ytimocin
force-pushed
the
ytimocin/air-gapped-cluster
branch
from
9e8db98 to
469cf36
Compare
ytimocin
had a problem deploying
to
functional-tests
GitHub Actions
Failure
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
ytimocin
force-pushed
the
ytimocin/air-gapped-cluster
branch
from
71a1d32 to
dfe0571
Compare
…he credentials in the terraformrc file. Signed-off-by: ytimocin <[email protected]>
ytimocin
force-pushed
the
ytimocin/air-gapped-cluster
branch
from
dfe0571 to
eede0c9
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
- Handle empty maps/arrays passed as strings in recipe parameters
- Add recursive normalization for nested structures with string-encoded empty objects
- Support whitespace variations (e.g., '{ }', '{}\n', '[ ]')
- Remove TF_LOG environment variable to prevent terraform-exec conflicts
- Add comprehensive test coverage for parameter normalization
Signed-off-by: ytimocin <[email protected]>
ytimocin
force-pushed
the
ytimocin/air-gapped-cluster
branch
from
f1615ee to
65e1f62
Compare
Signed-off-by: ytimocin <[email protected]>
Test feature Signed-off-by: Sylvain Niles <[email protected]>
brooke-hamilton
temporarily deployed
to
publish-bicep
GitHub Actions
Inactive
# Description Bugfix and adds support for injecting azurerm credentials when provider block is present. ## Type of change <!-- Please select **one** of the following options that describes your change and delete the others. Clearly identifying the type of change you are making will help us review your PR faster, and is used in authoring release notes. If you are making a bug fix or functionality change to Radius and do not have an associated issue link please create one now. --> - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional). ## Contributor checklist Please verify that the PR meets the following requirements, where applicable: <!-- This checklist uses "TaskRadio" comments to make certain options mutually exclusive. See: https://github.com/mheap/require-checklist-action?tab=readme-ov-file#radio-groups For details on how this works and why it's required. --> - An overview of proposed schema changes is included in a linked GitHub issue. - [ ] Yes <!-- TaskRadio schema --> - [x] Not applicable <!-- TaskRadio schema --> - A design document PR is created in the [design-notes repository](https://github.com/radius-project/design-notes/), if new APIs are being introduced. - [ ] Yes <!-- TaskRadio design-pr --> - [x] Not applicable <!-- TaskRadio design-pr --> - The design document has been reviewed and approved by Radius maintainers/approvers. - [ ] Yes <!-- TaskRadio design-review --> - [x] Not applicable <!-- TaskRadio design-review --> - A PR for the [samples repository](https://github.com/radius-project/samples) is created, if existing samples are affected by the changes in this PR. - [ ] Yes <!-- TaskRadio samples-pr --> - [x] Not applicable <!-- TaskRadio samples-pr --> - A PR for the [documentation repository](https://github.com/radius-project/docs) is created, if the changes in this PR affect the documentation or any user facing updates are made. - [ ] Yes <!-- TaskRadio docs-pr --> - [x] Not applicable <!-- TaskRadio docs-pr --> - A PR for the [recipes repository](https://github.com/radius-project/recipes) is created, if existing recipes are affected by the changes in this PR. - [ ] Yes <!-- TaskRadio recipes-pr --> - [x] Not applicable <!-- TaskRadio recipes-pr --> --------- Signed-off-by: Sylvain Niles <[email protected]>
# Description Sets the OS environment variable for git binary to use the provided cert. ## Type of change <!-- Please select **one** of the following options that describes your change and delete the others. Clearly identifying the type of change you are making will help us review your PR faster, and is used in authoring release notes. If you are making a bug fix or functionality change to Radius and do not have an associated issue link please create one now. --> - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional). <!-- Please update the following to link the associated issue. This is required for some kinds of changes (see above). --> ## Contributor checklist Please verify that the PR meets the following requirements, where applicable: <!-- This checklist uses "TaskRadio" comments to make certain options mutually exclusive. See: https://github.com/mheap/require-checklist-action?tab=readme-ov-file#radio-groups For details on how this works and why it's required. --> - An overview of proposed schema changes is included in a linked GitHub issue. - [ ] Yes <!-- TaskRadio schema --> - [x] Not applicable <!-- TaskRadio schema --> - A design document PR is created in the [design-notes repository](https://github.com/radius-project/design-notes/), if new APIs are being introduced. - [ ] Yes <!-- TaskRadio design-pr --> - [x] Not applicable <!-- TaskRadio design-pr --> - The design document has been reviewed and approved by Radius maintainers/approvers. - [ ] Yes <!-- TaskRadio design-review --> - [x] Not applicable <!-- TaskRadio design-review --> - A PR for the [samples repository](https://github.com/radius-project/samples) is created, if existing samples are affected by the changes in this PR. - [ ] Yes <!-- TaskRadio samples-pr --> - [x] Not applicable <!-- TaskRadio samples-pr --> - A PR for the [documentation repository](https://github.com/radius-project/docs) is created, if the changes in this PR affect the documentation or any user facing updates are made. - [ ] Yes <!-- TaskRadio docs-pr --> - [x] Not applicable <!-- TaskRadio docs-pr --> - A PR for the [recipes repository](https://github.com/radius-project/recipes) is created, if existing recipes are affected by the changes in this PR. - [ ] Yes <!-- TaskRadio recipes-pr --> - [x] Not applicable <!-- TaskRadio recipes-pr -->
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+455
to
+482
| for _, f := range r.File { | ||
| fp := filepath.Join(destDir, f.Name) | ||
| if f.FileInfo().IsDir() { | ||
| if err := os.MkdirAll(fp, 0755); err != nil { | ||
| return err | ||
| } | ||
| continue | ||
| } | ||
| if err := os.MkdirAll(filepath.Dir(fp), 0755); err != nil { | ||
| return err | ||
| } | ||
| rc, err := f.Open() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| out, err := os.OpenFile(fp, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, f.Mode()) | ||
| if err != nil { | ||
| rc.Close() | ||
| return err | ||
| } | ||
| if _, err := io.Copy(out, rc); err != nil { | ||
| out.Close() | ||
| rc.Close() | ||
| return err | ||
| } | ||
| out.Close() | ||
| rc.Close() | ||
| } |
Check failure
Code scanning / CodeQL
Arbitrary file access during archive extraction ("Zip Slip") High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
To fix this issue, we must ensure that files extracted from zip entries are only written beneath the intended destDir and cannot escape via directory traversal (..) or absolute paths. The best way to do this is to construct the full output path for each entry, then check that its normalized (absolute) path remains under destDir. The easy way is to use filepath.Join(destDir, f.Name) to construct the target path, then resolve both destDir and the target path to their absolute forms (with filepath.Abs), and check that the target path starts with the destination directory’s path.
Additionally, we should reject absolute paths, or any entry with .. components in its name. We should skip any such entry, or fail the extraction.
You only need to edit the unzip function in pkg/recipes/driver/terraform/registry.go. To implement the fix:
- Import
path/filepathand use itsAbsandCleanfunctions to compute absolute paths. - Before creating directories or files, validate the path:
- Get the absolute path of
destDirandfp. - Ensure the absolute path of the file starts with the absolute path to the output directory, with a path separator after the directory to avoid prefix issues.
- Optionally, skip entries whose names include
..or start with a separator (i.e., are absolute).
- Get the absolute path of
- If the check fails, skip the entry or return an error.
- No new dependencies are required.
Edits will be confined to the unzip function code block.
Suggested changeset
1
pkg/recipes/driver/terraform/registry.go
| @@ -373,22 +373,37 @@ | ||
| return err | ||
| } | ||
| defer r.Close() | ||
| absDestDir, err := filepath.Abs(destDir) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| for _, f := range r.File { | ||
| fp := filepath.Join(destDir, f.Name) | ||
| // Prevent Zip Slip by cleaning and checking the filepath | ||
| fp := filepath.Join(absDestDir, f.Name) | ||
| fp = filepath.Clean(fp) | ||
| absFP, err := filepath.Abs(fp) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| // The filepath must start with the destination directory + separator to prevent prefix matches | ||
| // Also ensure f.Name does not contain ".." or is absolute | ||
| if !strings.HasPrefix(absFP, absDestDir+string(os.PathSeparator)) && absFP != absDestDir { | ||
| return fmt.Errorf("illegal file path in zip: %s", f.Name) | ||
| } | ||
| if f.FileInfo().IsDir() { | ||
| if err := os.MkdirAll(fp, 0755); err != nil { | ||
| if err := os.MkdirAll(absFP, 0755); err != nil { | ||
| return err | ||
| } | ||
| continue | ||
| } | ||
| if err := os.MkdirAll(filepath.Dir(fp), 0755); err != nil { | ||
| if err := os.MkdirAll(filepath.Dir(absFP), 0755); err != nil { | ||
| return err | ||
| } | ||
| rc, err := f.Open() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| out, err := os.OpenFile(fp, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, f.Mode()) | ||
| out, err := os.OpenFile(absFP, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, f.Mode()) | ||
| if err != nil { | ||
| rc.Close() | ||
| return err |
Copilot is powered by AI and may make mistakes. Always verify output.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This pull request introduces significant enhancements and new features across multiple areas, including Kubernetes deployment, configuration management, and development environment initialization. Key changes include adding support for air-gapped environments, refining Kubernetes installation options, and improving the handling of provisioning states in resources.
Kubernetes Deployment Enhancements:
pkg/cli/cmd/install/kubernetes/kubernetes.go). [1] [2] [3] [4]deploy-local.shscript to automate building, pushing Docker images, resetting the Kubernetes cluster, and publishing Bicep extensions (deploy-local.sh).Air-Gapped Environment Support:
--recipe-registryflag in therad initcommand to specify an alternative container registry for dev recipes (pkg/cli/cmd/radinit/init.go). [1] [2] [3] [4]GetDevRecipesmethod to accept arecipeRegistryparameter for fetching recipes from a custom registry (pkg/cli/cmd/radinit/mock_devrecipeclient.go,pkg/cli/cmd/radinit/environment.go). [1] [2] [3]Configuration Improvements:
app-kubernetes-postgres.bicepfile for managing secrets, authentication, and registry settings (app-kubernetes-postgres.bicep).bicepconfig.jsonfile to use a local extension path instead of a registry reference (bicepconfig.json).Resource Management:
ProvisioningStatemethods inBaseResourceto use a simpler structure (pkg/armrpc/api/v1/types.go).Testing and Mock Updates:
recipeRegistryparameter (pkg/cli/cmd/radinit/init_test.go,pkg/cli/cmd/radinit/mock_devrecipeclient.go). [1] [2] [3]Type of change
Fixes: #issue_number
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: