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.

Sign up for GitHub

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

Jump to bottom

Allows build/run UIDs to differ #28

Merged
merged 1 commit into from
Jul 18, 2022
Merged

Allows build/run UIDs to differ #28

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
0 config/nginx-fpm.conf → assets/nginx-fpm.conf
View file
Open in desktop
File renamed without changes.
2 changes: 2 additions & 0 deletions config/nginx.conf → assets/nginx.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ events {
worker_connections 1024;
}

pid /tmp/nginx.pid;

http {
types {
text/html html htm shtml;
Expand Down
6 changes: 3 additions & 3 deletions build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
// ConfigWriter sets up the default Nginx configuration file and incorporates
// any user configurations.
type ConfigWriter interface {
Write(layerPath, workingDir, cnbPath string) (string, error)
Write(workingDir string) (string, error)
}

// Build will return a packit.BuildFunc that will be invoked during the build
Expand Down Expand Up @@ -39,14 +39,14 @@ func Build(nginxConfigWriter ConfigWriter, nginxFpmConfigWriter ConfigWriter, lo
}

logger.Process("Setting up the Nginx configuration file")
nginxConfigPath, err := nginxConfigWriter.Write(phpNginxLayer.Path, context.WorkingDir, context.CNBPath)
nginxConfigPath, err := nginxConfigWriter.Write(context.WorkingDir)
if err != nil {
return packit.BuildResult{}, err
}
logger.Break()

logger.Process("Setting up the Nginx-specific FPM configuration file")
_, err = nginxFpmConfigWriter.Write(phpNginxLayer.Path, context.WorkingDir, context.CNBPath)
_, err = nginxFpmConfigWriter.Write(context.WorkingDir)
if err != nil {
return packit.BuildResult{}, err
}
Expand Down
5 changes: 0 additions & 5 deletions build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,8 @@ func testBuild(t *testing.T, context spec.G, it spec.S) {
result, err := build(buildContext)
Expect(err).NotTo(HaveOccurred())

Expect(nginxConfigWriter.WriteCall.Receives.LayerPath).To(Equal(filepath.Join(layerDir, "php-nginx-config")))
Expect(nginxConfigWriter.WriteCall.Receives.WorkingDir).To(Equal(workingDir))
Expect(nginxConfigWriter.WriteCall.Receives.CnbPath).To(Equal(cnbDir))

Expect(nginxFpmConfigWriter.WriteCall.Receives.LayerPath).To(Equal(filepath.Join(layerDir, "php-nginx-config")))
Expect(nginxFpmConfigWriter.WriteCall.Receives.WorkingDir).To(Equal(workingDir))
Expect(nginxFpmConfigWriter.WriteCall.Receives.CnbPath).To(Equal(cnbDir))

Expect(result.Layers).To(HaveLen(1))
Expect(result.Layers[0]).To(Equal(expectedPhpNginxLayer))
Expand Down
2 changes: 1 addition & 1 deletion buildpack.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ api = "0.7"
uri = "https://github.com/paketo-buildpacks/php-nginx/blob/main/LICENSE"

[metadata]
include-files = ["bin/build", "bin/detect", "bin/run", "buildpack.toml", "config/nginx.conf", "config/nginx-fpm.conf"]
include-files = ["bin/build", "bin/detect", "bin/run", "buildpack.toml"]
pre-package = "./scripts/build.sh"

[[stacks]]
Expand Down
57 changes: 38 additions & 19 deletions config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package phpnginx

import (
"bytes"
_ "embed"
"fmt"
"io"
"os"
"path/filepath"
"strconv"
Expand All @@ -12,6 +12,12 @@ import (
"github.com/paketo-buildpacks/packit/v2/scribe"
)

//go:embed assets/nginx.conf
var NGINXConfTemplate string

//go:embed assets/nginx-fpm.conf
var NGINXFPMConfTemplate string

type NginxConfig struct {
UserServerConf string
UserHttpConf string
Expand Down Expand Up @@ -45,8 +51,8 @@ func NewFpmNginxConfigWriter(logger scribe.Emitter) NginxFpmConfigWriter {
}
}

func (c NginxConfigWriter) Write(layerPath, workingDir, cnbPath string) (string, error) {
tmpl, err := template.New("nginx.conf").ParseFiles(filepath.Join(cnbPath, "config", "nginx.conf"))
func (c NginxConfigWriter) Write(workingDir string) (string, error) {
tmpl, err := template.New("nginx.conf").Parse(NGINXConfTemplate)
if err != nil {
return "", fmt.Errorf("failed to parse Nginx config template: %w", err)
}
Expand All @@ -55,6 +61,7 @@ func (c NginxConfigWriter) Write(layerPath, workingDir, cnbPath string) (string,
AppRoot: workingDir,
}

var configFiles []string
// Configuration set by this buildpack
// If there's a user-provided Nginx conf, include it in the base configuration.
userServerConf := filepath.Join(workingDir, ".nginx.conf.d", "*-server.conf")
Expand All @@ -65,6 +72,7 @@ func (c NginxConfigWriter) Write(layerPath, workingDir, cnbPath string) (string,
}
if len(userServerMatches) > 0 {
data.UserServerConf = userServerConf
configFiles = append(configFiles, userServerMatches...)
c.logger.Debug.Subprocess(fmt.Sprintf("Including user-provided Nginx server configuration from: %s", userServerConf))
}

Expand All @@ -76,6 +84,7 @@ func (c NginxConfigWriter) Write(layerPath, workingDir, cnbPath string) (string,
}
if len(userHttpMatches) > 0 {
data.UserHttpConf = userHttpConf
configFiles = append(configFiles, userHttpMatches...)
c.logger.Debug.Subprocess(fmt.Sprintf("Including user-provided Nginx HTTP configuration from: %s", userHttpConf))
}

Expand All @@ -97,7 +106,7 @@ func (c NginxConfigWriter) Write(layerPath, workingDir, cnbPath string) (string,
data.DisableHTTPSRedirect = !enableHTTPSRedirect
c.logger.Debug.Subprocess(fmt.Sprintf("Enable HTTPS redirect: %t", enableHTTPSRedirect))

fpmSocket := filepath.Join(layerPath, "php-fpm.socket")
fpmSocket := "/tmp/php-fpm.socket"
data.FpmSocket = fpmSocket
c.logger.Debug.Subprocess(fmt.Sprintf("FPM socket: %s", fpmSocket))

Expand All @@ -108,31 +117,37 @@ func (c NginxConfigWriter) Write(layerPath, workingDir, cnbPath string) (string,
return "", err
}

f, err := os.OpenFile(filepath.Join(workingDir, "nginx.conf"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm)
path := filepath.Join(workingDir, "nginx.conf")
err = os.WriteFile(path, b.Bytes(), 0600)
Copy link
Member

Choose a reason for hiding this comment

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

Why does the nginx.conf have 0600 permissions, but the .php.fpm.bp/nginx-fpm.conf on line 164 have 0660?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. I was relying on some coincidental configuration of umask on my machine that I should instead explicitly force with chmod.

As for the explanation, we should really only set 0x00 permissions on files using os.WriteFile, where x is the permission bit for the current user. Then we should use os.Chmod to force the permissions to a specific value. The reason for doing this is that os.WriteFile uses umask to mask the permissions bits when creating files. This could mean that even if you say os.WriteFile(path, bytes, 0777), you could see the file get permissions of 0755 applied if you have 0022 set as the umask.

I think this StackOverflow post helps explain the phenomenon: https://stackoverflow.com/questions/50257981/ioutils-writefile-not-respecting-permissions

Copy link
Member

Choose a reason for hiding this comment

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

This is so helpful!! Thank you for the details, I hadn't heard of that before

Copy link
Member

Choose a reason for hiding this comment

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

Relatedly, info.Mode()|0040 this line adds the group read permissions onto the existing permissions?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes. this is a bitwise OR operation. It will set the "read bit" for the group to 1.

if err != nil {
return "", err
}
defer f.Close()

_, err = io.Copy(f, &b)
if err != nil {
// not tested
return "", err
for _, file := range append(configFiles, path) {
info, err := os.Stat(file)
if err != nil {
return "", err
}

err = os.Chmod(file, info.Mode()|0060)
if err != nil {
return "", err
}
}

return f.Name(), nil
return path, nil
}

func (c NginxFpmConfigWriter) Write(layerPath, workingDir, cnbPath string) (string, error) {
tmpl, err := template.New("nginx-fpm.conf").ParseFiles(filepath.Join(cnbPath, "config", "nginx-fpm.conf"))
func (c NginxFpmConfigWriter) Write(workingDir string) (string, error) {
tmpl, err := template.New("nginx-fpm.conf").Parse(NGINXFPMConfTemplate)
if err != nil {
return "", fmt.Errorf("failed to parse Nginx-Fpm config template: %w", err)
}

// Configuration set by this buildpack

// If there's a user-provided Nginx conf, include it in the base configuration.
fpmSocket := filepath.Join(layerPath, "php-fpm.socket")
fpmSocket := "/tmp/php-fpm.socket"
c.logger.Debug.Subprocess(fmt.Sprintf("FPM socket: %s", fpmSocket))

data := NginxFpmConfig{
Expand All @@ -146,17 +161,21 @@ func (c NginxFpmConfigWriter) Write(layerPath, workingDir, cnbPath string) (stri
return "", err
}

f, err := os.OpenFile(filepath.Join(workingDir, ".php.fpm.bp", "nginx-fpm.conf"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm)
path := filepath.Join(workingDir, ".php.fpm.bp", "nginx-fpm.conf")
err = os.WriteFile(path, b.Bytes(), 0600)
if err != nil {
return "", err
}
defer f.Close()

_, err = io.Copy(f, &b)
info, err := os.Stat(path)
if err != nil {
return "", err
}

err = os.Chmod(path, info.Mode()|0040)
if err != nil {
// not tested
return "", err
}

return f.Name(), nil
return path, nil
}
Loading