certificate propagation throughout the build process#647
certificate propagation throughout the build process#647jay-rah-oob wants to merge 1 commit intosuse-edge:mainfrom
Conversation
ipetrov117
left a comment
ipetrov117
left a comment
There was a problem hiding this comment.
Hey @one-horned-flying and thank you for your contribution! 🚀
Could I ask you to describe the problem that you hit in an Issue and link it to this PR as well, thanks!
Apart from the review comments below, I believe we should not bundle the certificates for both the EIB-built image and the RPM resolution container together. The certificate requirements for these two components may differ, and bundling them could lead to unnecessary certificates being included in either the RPM resolution container or the EIB-built image. This could introduce unexpected issues.
In this PR, I suggest to implement the following image configuration directory structure enhancement:
.
├── ...
├── certificates
│ ├── custom-image-ca.crt
│ └── custom-image-ca.pem
├── ...
└── rpms
└── certificates
├── rpm-resolution-ca.crt
└── rpm-resolution-ca.pemWhere:
certificates- this directory will continue to only be used for certificates related to the custom image that EIB is building.rpms/certificates- new directory that will hold certificates that will only be propagated to theRPM resolutioncontainer.
Doing this will ensure that no unnecessary certificates are placed in the corresponding containers/images, which will mitigate any possible unforeseen problems. Let me know what you think. CC: @atanasdinov
| @@ -1,6 +1,9 @@ | |||
| # ----- EIB Builder Image ----- | |||
There was a problem hiding this comment.
There is no need to add the certificates as part of the EIB Dockerfile itself. The EIB Dockerfile is running a fixed set of configurations that are proven to successfully build an EIB container image.
Any configurations for EIB or its underlying RPM resolution process must be done only from EIB's image configuration directory.
Based on this I suggest we remove the certificate inclusion from here.
There was a problem hiding this comment.
In my environment, the outgoing traffic is reencrypted at the edge and monitored, that means I have a cert that I need to add to the trust for building of the base EIB image. When using the upstream image it is pretty simple to just add my needed certs though.
There was a problem hiding this comment.
basically an accepted MITM :)
| @@ -0,0 +1 @@ | |||
| * | |||
There was a problem hiding this comment.
Same as the comment above, can we remove this file and directory. EIB configurations should be done only from EIB's image configuration directory.
|
|
||
| type mockRPMResolver struct { | ||
| resolveFunc func(packages *image.Packages, localRPMConfig *image.LocalRPMConfig, outputDir string) (rpmDir string, pkgList []string, err error) | ||
| resolveFunc func(packages *image.Packages, localRPMConfig *image.LocalRPMConfig, certsPath, outputDir string) (rpmDir string, pkgList []string, err error) |
There was a problem hiding this comment.
This change needs to be reflected in the resolveFunc functions of the TestConfigureRPMs_ResolutionFailures and the TestConfigureRPMs_SuccessfulConfig test cases.
| return fmt.Errorf("creating certificates dir %s: %w", certsDest, err) | ||
| } | ||
|
|
||
| if _, err := os.Stat(certsPath); os.IsNotExist(err) { |
There was a problem hiding this comment.
I suggest moving the validation before the actual resolver-image-build/certificates directory creation. No need to create a directory if the root certificates directory is empty.
Also we could probably use the existing fileio.FileExists function here.
| {{ end -}} | ||
| {{ end }} | ||
|
|
||
| COPY certificates/. /etc/pki/trust/anchors/ |
There was a problem hiding this comment.
I'd suggest to avoid hardcoding the path to the certificates directory in the Dockerfile.tpl file. Instead pass the path from the code and have a template here as it is for the other properties. That way if a change needs to happen it could only be done in a single place. Templating for the resolver's Dockerfile.tpl is happening in the writeDockerfile function.
2 participants
During the "Resolving package dependencies..." phase the Dockerfile does not contain the certificates which are put in the
certificatesfolder. This adds those certs so that the rest of the build process can succeed.